#metabrainz

But it works fine for now

LordSputnik: When we drop the Bookshelf, then we simply replace the `bookshelf(knex(config ))` by `knex(config)`

That's a package-wide issue, not specific to func, so don't worry about it :)

LordSputnik: Ok

Great

I saw your SQL PR

Looks good to me, just a few comments I added

I also read through the relationship plan properly, and think that you've got the optimal way of doing it there

So am happy with that

(at least, I can't come up with a good alternative, and it seems sensible!)

LordSputnik: Ok, great!

LordSputnik: One thing I was wondering, now that we are working on the adding function based transactional approach, do I write functions for adding alias and alias_set and everything else, or simply reuse the present bookshelf based approach

bukwurm: I'd say stick with the ORM where other bits of the site use it

LordSputnik: Ok, that's a relief, because I was getting worried as it would have been tough.

Adding functions for interacting with the rest of the database is going to be a fair amount of work, and I think that you should focus on the import stuff mostly

Yeah exactly

I'm planning to convert gradually over the course of a few months, so once I have functions I might go in and swap over some of the database access you've written using the ORM, but definitely stick with the ORM to begin with

LordSputnik: Ok, I'll need a little bit of your help figuring out how to fit in the ORM based functions though :)

Hmm

You could pass the ORM into the func.init function

Ok, I'll try that out.

I don't think you'll have any issues mixing knex and bookshelf objects, since the functions in the ORM (e.g. save(), fetch()) will just be calling knex underneath, and you can pass transactions into those functions

LordSputnik: I'll might need repositories for the remaining part of the project sometime this week.

LordSputnik: Ok

bukwurm: ah yes, sorry bout that

I'll aim to do it tonight

I'm working on some GDPR stuff (account removal) for BookBrainz so I'll do it at the end of that

LordSputnik: Ok

bukwurm: learnt any interesting things this week that you'd like to share? :)

LordSputnik: Well, I figured out setting up the library and got a bit more comfortable with knex and flow.

I did quite a lot of reading on rabbitmq

Good :) Do you think I'm taking the right approach with Immutable records?

LordSputnik: Yeah, they're very similar to js objects and so will be perfect for setting up props and other stuff.

Plus, default values are a plus.

Good, glad you agree, I was a bit unsure at first but I think I'm convinced now

Is rabbitmq simple?

LordSputnik: Yeah it's not that difficult I guess.

I don't think I've used it before

The armq.node library is quite straightforward.

Although I have used similar things

But I'm still reading it, so can't be sure right now.

LordSputnik: Ok

I might try setting it up on the BookBrainz server next week

LordSputnik: I found bookshelf very non-intuitive, so it's easier compared to that. :)

LordSputnik: Similar like kafka?

I think SQLAlchemy in Python is much nicer than bookshelf in JS

LordSputnik: +1

LordSputnik: I have been reading around regarding choice of ORM for node

bukwurm: can't remember exactly what it was I used, but it was in an IoT system - edge nodes sent messages to the central server

And Objection.js has a good reputation.

LordSputnik: Ok. I used a similar thing in ROS project.

It probably didn't exist when we picked bookshelf about 4 years ago!

LordSputnik: Exactly what I thought when I saw Objection

:)

I'm pretty sure that moving away from an ORM and towards functions is the right thing though

LordSputnik: Me too. ORMs have some pretty tough restrictions.

bukwurm: any challenges you can see coming up over the next week?

LordSputnik: I am terribly behind schedule, so that's the biggest problem right now. 😓

All this work had to be completed by today.

Still, I'll try to catch up this week.

LordSputnik: Another request I have is could you please add nodemon as a dependency

in the bb-data ?

Also, flow and flow-bin are missing as dependencies.

bukwurm: I think as long as you can get the first package of work done by the end of next week, and started on the producer application, you'll be on track

LordSputnik: It'll be great to have nodemon run prepublish silently in the background and not having to do it manually every time.

LordSputnik: Yeah, I am thinking of the same. :)

LordSputnik: One more important thing I almost misssed

knex takes in regular js objects

as parameters, and none of Immutable.js objects

bukwurm: should be able to do a PR for that - essentially just update package.json to have all the dependencies you want to commit, then delete node_modules and the package lock file, then "npm install", then commit the package and lock JSON files

(the dependencies)

LordSputnik: I'd do it, but will it be ok to mess up the lock file?

bukwurm: it should just update some of the locked packages

LordSputnik: npm behaves weird in this regard

I don't know if you remember

Essentially we're mainly using them to ensure that the version tested by CI and the versions we deploy in production are the same

But couple of months ago I brought it up here

If I simply clone the bb-site repo and run npm install

Yeah, it's not great. But if you delete node_modules and package-lock.json before doing npm install, I've found it generally works ok

It changes the lock file!

Yeah, it's probably silently updating the package versions when a new minor version has been released

LordSputnik: But it means it'll tamper with git and everything, and I'll have to ignore it.

It's OK to change the lock file :)

Also, I read some issue where it adds system specific dependencies

Like in mac systems it adds fs-events or something

Alternatively you can just update package.json in the PR, then I can make another commit afterwards updating the locked packages

LordSputnik: Anyhow, I'll try it

Let's see if it makes some weird changes.

LordSputnik: Ok, that's work for me. :)

*that'll

Back to the knex stuff?

LordSputnik: knex takes in regular js objects as parameters, and none of Immutable.js objects

LordSputnik: Yeah

So there are two choices

Either we pass simple js objects to all bb-data function, but they return Immutable objects

It would be nice if BB could move to yarn instead of npm (shouldn't take much time to port)

samj1912: tried that, it worked worse than npm

How so?

And when?

Or we pass Immutable objects which are then converted to simple js objects

Used in the query and then returned back as Immutable objects

samj1912: https://github.com/bookbrainz/bookbrainz-site/c...

samj1912: Hi I read your message but I was horribly late so I didn't reply

bukwurm: I like the second approach

Perhaps have a function for each record that converts to/from database format

LordSputnik: Ok

11 months huh, yarn has improved considerably

LordSputnik: But wouldn't it add unnecessary overhead?

I started using the second approach in getOrCreateAlias here: https://github.com/bookbrainz/bookbrainz-data-j...

Plus it's much more faster

bukwurm: the time taken to access the database is probably much longer than the time taken to convert to/from a Record

LordSputnik: Oh, I missed that 😅

LordSputnik: Definitely

And I think it's nice to present a consistent interface to package users - everything in or out is an immutable object

LordSputnik: I am making the addImport functions take in an array of importData objects

LordSputnik: Yeah that's definitely a great thing

So in your case you could use an Immutable List for that

Or a set

LordSputnik: Yeah that was a the thing of discussion

List sounds more intuitive

Maybe, although if you have the same data object twice, you probably would only want to deal with it once, so a set might be more appropriate

LordSputnik: Yeah that's also an important point

Lets stick with set, as I would rather have set than have transactions rolling back due to redundancy problems

bukwurm: ok, anything else you'd like to discuss before we close for today? Are you generally happy and enjoying the project so far?

LordSputnik: Nothing else right now. I'll write up the next weeks plan shortly.

OK good :)

LordSputnik: The projects is great and next week would be far more exciting I guess! :D

Just need to get over with the access methods :)

I'm probably going to be a little bit busier next week, so do you mind us having our shorter meetings on Tuesday and Thursday evening (UK time) only?

LordSputnik: Sure

Great :)

I'll aim to get your repositories set up tonight

Good talking :D

LordSputnik: Thank you for clearing up things. I'll try my best to get back on track this week.