#metabrainz

/

      • LordSputnik
        But it works fine for now
      • bukwurm
        LordSputnik: When we drop the Bookshelf, then we simply replace the `bookshelf(knex(config ))` by `knex(config)`
      • LordSputnik
        That's a package-wide issue, not specific to func, so don't worry about it :)
      • bukwurm
        LordSputnik: Ok
      • Great
      • LordSputnik
        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!)
      • bukwurm
        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
      • LordSputnik
        bukwurm: I'd say stick with the ORM where other bits of the site use it
      • bukwurm
        LordSputnik: Ok, that's a relief, because I was getting worried as it would have been tough.
      • LordSputnik
        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
      • bukwurm
        LordSputnik: Ok, I'll need a little bit of your help figuring out how to fit in the ORM based functions though :)
      • LordSputnik
        Hmm
      • You could pass the ORM into the func.init function
      • And then pass it to whatever functions need it
      • bukwurm
        Ok, I'll try that out.
      • LordSputnik
        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
      • bukwurm
        LordSputnik: I'll might need repositories for the remaining part of the project sometime this week.
      • LordSputnik: Ok
      • LordSputnik
        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
      • bukwurm
        LordSputnik: Ok
      • LordSputnik
        bukwurm: learnt any interesting things this week that you'd like to share? :)
      • bukwurm
        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
      • LordSputnik
        Good :) Do you think I'm taking the right approach with Immutable records?
      • bukwurm
        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.
      • LordSputnik
        Good, glad you agree, I was a bit unsure at first but I think I'm convinced now
      • Is rabbitmq simple?
      • bukwurm
        LordSputnik: Yeah it's not that difficult I guess.
      • LordSputnik
        I don't think I've used it before
      • bukwurm
        The armq.node library is quite straightforward.
      • LordSputnik
        Although I have used similar things
      • bukwurm
        But I'm still reading it, so can't be sure right now.
      • LordSputnik: Ok
      • LordSputnik
        I might try setting it up on the BookBrainz server next week
      • bukwurm
        LordSputnik: I found bookshelf very non-intuitive, so it's easier compared to that. :)
      • LordSputnik: Similar like kafka?
      • LordSputnik
        I think SQLAlchemy in Python is much nicer than bookshelf in JS
      • bukwurm
        LordSputnik: +1
      • LordSputnik: I have been reading around regarding choice of ORM for node
      • LordSputnik
        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
      • bukwurm
        And Objection.js has a good reputation.
      • LordSputnik: Ok. I used a similar thing in ROS project.
      • LordSputnik
        It probably didn't exist when we picked bookshelf about 4 years ago!
      • bukwurm
        LordSputnik: Exactly what I thought when I saw Objection
      • :)
      • LordSputnik
        I'm pretty sure that moving away from an ORM and towards functions is the right thing though
      • bukwurm
        LordSputnik: Me too. ORMs have some pretty tough restrictions.
      • LordSputnik
        bukwurm: any challenges you can see coming up over the next week?
      • bukwurm
        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.
      • LordSputnik
        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
      • bukwurm
        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
      • LordSputnik
        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)
      • bukwurm
        LordSputnik: I'd do it, but will it be ok to mess up the lock file?
      • LordSputnik
        bukwurm: it should just update some of the locked packages
      • bukwurm
        LordSputnik: npm behaves weird in this regard
      • I don't know if you remember
      • LordSputnik
        Essentially we're mainly using them to ensure that the version tested by CI and the versions we deploy in production are the same
      • bukwurm
        But couple of months ago I brought it up here
      • If I simply clone the bb-site repo and run npm install
      • LordSputnik
        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
      • bukwurm
        It changes the lock file!
      • LordSputnik
        Yeah, it's probably silently updating the package versions when a new minor version has been released
      • bukwurm
        LordSputnik: But it means it'll tamper with git and everything, and I'll have to ignore it.
      • LordSputnik
        It's OK to change the lock file :)
      • bukwurm
        Also, I read some issue where it adds system specific dependencies
      • Like in mac systems it adds fs-events or something
      • LordSputnik
        Alternatively you can just update package.json in the PR, then I can make another commit afterwards updating the locked packages
      • bukwurm
        LordSputnik: Anyhow, I'll try it
      • Let's see if it makes some weird changes.
      • LordSputnik: Ok, that's work for me. :)
      • *that'll
      • LordSputnik
        Back to the knex stuff?
      • bukwurm
        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
      • samj1912
        It would be nice if BB could move to yarn instead of npm (shouldn't take much time to port)
      • LordSputnik
        samj1912: tried that, it worked worse than npm
      • samj1912
        How so?
      • And when?
      • bukwurm
        Or we pass Immutable objects which are then converted to simple js objects
      • Used in the query and then returned back as Immutable objects
      • LordSputnik
      • bukwurm
        samj1912: Hi I read your message but I was horribly late so I didn't reply
      • LordSputnik
        bukwurm: I like the second approach
      • Perhaps have a function for each record that converts to/from database format
      • bukwurm
        LordSputnik: Ok
      • samj1912
        11 months huh, yarn has improved considerably
      • bukwurm
        LordSputnik: But wouldn't it add unnecessary overhead?
      • LordSputnik
        I started using the second approach in getOrCreateAlias here: https://github.com/bookbrainz/bookbrainz-data-j...
      • samj1912
        Plus it's much more faster
      • LordSputnik
        bukwurm: the time taken to access the database is probably much longer than the time taken to convert to/from a Record
      • bukwurm
        LordSputnik: Oh, I missed that 😅
      • LordSputnik: Definitely
      • LordSputnik
        And I think it's nice to present a consistent interface to package users - everything in or out is an immutable object
      • bukwurm
        LordSputnik: I am making the addImport functions take in an array of importData objects
      • LordSputnik: Yeah that's definitely a great thing
      • LordSputnik
        So in your case you could use an Immutable List for that
      • Or a set
      • bukwurm
        LordSputnik: Yeah that was a the thing of discussion
      • List sounds more intuitive
      • LordSputnik
        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
      • bukwurm
        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_ has quit
      • vikrantprasad5[m has quit
      • sfunk1x has quit
      • adhawkins has quit
      • LordSputnik
        bukwurm: ok, anything else you'd like to discuss before we close for today? Are you generally happy and enjoying the project so far?
      • adhawkins joined the channel
      • adhawkins has quit
      • adhawkins joined the channel
      • yagyanshbhatia[m has quit
      • dns[m] has quit
      • GregKNicholson[m has quit
      • ashwanig has quit
      • jwf has quit
      • alastairp has quit
      • alastairp joined the channel
      • bukwurm
        LordSputnik: Nothing else right now. I'll write up the next weeks plan shortly.
      • LordSputnik
        OK good :)
      • bukwurm
        LordSputnik: The projects is great and next week would be far more exciting I guess! :D
      • ashwanig joined the channel
      • Just need to get over with the access methods :)
      • LordSputnik
        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?
      • bukwurm
        LordSputnik: Sure
      • LordSputnik
        Great :)
      • I'll aim to get your repositories set up tonight
      • Good talking :D
      • bukwurm
        LordSputnik: Thank you for clearing up things. I'll try my best to get back on track this week.