akshat: monkey: front end tests are failing. can one of you look, please?
2021-10-25 29821, 2021
monkey
Looking at it now
2021-10-25 29858, 2021
akshat
ruaok I think we should add the links to our discussions on IRC regarding huesound to the PR description as well
2021-10-25 29805, 2021
akshat
Yes on it
2021-10-25 29846, 2021
ruaok
akshat: please do.
2021-10-25 29855, 2021
akshat
Okaay!
2021-10-25 29804, 2021
monkey
`Test Suites: 26 failed` definitely something wrong going on with the front-end tests.
2021-10-25 29844, 2021
monkey
rerunning the tests
2021-10-25 29846, 2021
Etua joined the channel
2021-10-25 29821, 2021
akshat
There is some problem with an import monkey I was trying to figure out how to get rid of it
2021-10-25 29833, 2021
akshat
Do you have any better takes at resolving it?
2021-10-25 29803, 2021
monkey
Not sure what was happening, but there's tons of errors in code that we haven't touched in the PR
2021-10-25 29828, 2021
monkey
Lots of symptoms, but not sure what the cause is
2021-10-25 29829, 2021
akshat
I bumped up the dependencies. That has caused a change
2021-10-25 29840, 2021
akshat
That isn't the major error though
2021-10-25 29849, 2021
akshat
But I'm pretty sure it is causing a few extra
2021-10-25 29859, 2021
akshat
What's the next step to bumping dependencies monkey?
2021-10-25 29813, 2021
monkey
Ah, I see. Which dependencies?
2021-10-25 29825, 2021
akshat
Quite a lot in package.json
2021-10-25 29802, 2021
monkey
Ah, yes.
2021-10-25 29828, 2021
akshat
Do we revert that or we just need a few additions while doing this?
2021-10-25 29831, 2021
monkey
What was the reason for this package update?
2021-10-25 29856, 2021
monkey
Yeah, I think we'll need to revert those changes and only update any package that's absolutely required
2021-10-25 29818, 2021
monkey
(and also package-lock.json changes)
2021-10-25 29856, 2021
monkey
In general, we only want to update one dependency at a time, or a set of related dependencies.
2021-10-25 29802, 2021
akshat
I was trying to resolve the import issue from an external package and in the meantime updated my local packages. The bump I made to dependecies work fine when testing locally but I do agree that it isn't good to do this in this pr
2021-10-25 29842, 2021
akshat
But asking in general, if I make a new pr and want to bump a lot of dependecies and want the tests to pass, what additional step should I make?
2021-10-25 29858, 2021
akshat
You can answer this later once we have dealt with current work 😃
2021-10-25 29840, 2021
monkey
Step one is don't bump a lot of dependencies :p We do it carefully one functionality at a time, otherwise it's harder to figure out what is happening if something breaks, and can hide other issues too.
2021-10-25 29853, 2021
akshat
Makes sense
2021-10-25 29824, 2021
monkey
And then I guess we deploy to test.lb if we want to test in a docker image in an environment similar to prod, and then check that the specific functionality you're changing is working as expected
2021-10-25 29854, 2021
akshat
Ah okayy
2021-10-25 29812, 2021
monkey
So for huesound, I'll let you first revert the changes to package.json and package-lock.json, and then we can look and see if there are any package issues (the one you were trying to resolve)
2021-10-25 29833, 2021
monkey
package import issues*
2021-10-25 29805, 2021
akshat
Reverted
2021-10-25 29840, 2021
monkey
OK, I'll pull the changes and see how it looks locally
2021-10-25 29846, 2021
akshat
Great!
2021-10-25 29854, 2021
yvanzo
zas: Thanks. What do you think of having a directory scripts/lib/ to distinguish scripts to be sourced only rather than executed?
2021-10-25 29823, 2021
zas
we can do that, I'm working on it atm
2021-10-25 29832, 2021
monkey
Looking at the front-end tests failing, I'm guessing this is the issue you were trying to solve akshat ?
the << master, HEAD >> stuff in the file. a failed merge perhaps?
2021-10-25 29822, 2021
monkey
Can't see the diff, which file is that?
2021-10-25 29840, 2021
monkey
Oh, I see it now
2021-10-25 29844, 2021
lucifer
package-lock.json
2021-10-25 29846, 2021
monkey
in package-lock
2021-10-25 29849, 2021
lucifer
yup
2021-10-25 29855, 2021
monkey
Yeah, that definitely looks wrong
2021-10-25 29836, 2021
TOPIC: MetaBrainz Community and Development channel | MusicBrainz non-development: #musicbrainz | BookBrainz: #bookbrainz | Channel is logged; see https://musicbrainz.org/doc/IRC for details | Agenda: Reviews, Follow up on Summit Notes -> Tickets, Allow test editing without verification (Freso/reo), Allow account admins to see editor-editor subscriptions? (Freso), next meeting PSA (Freso)
2021-10-25 29843, 2021
TOPIC: MetaBrainz Community and Development channel | MusicBrainz non-development: #musicbrainz | BookBrainz: #bookbrainz | Channel is logged; see https://musicbrainz.org/doc/IRC for details | Agenda: Reviews, Follow up on Summit Notes -> Tickets, Allow test editing without verification (Freso/reo), Allow account admins to see editor-editor subscriptions? (Freso), next meeting PSA (Freso)
2021-10-25 29829, 2021
monkey
Fixed. Still having issues locally though, seems unrelated
OK akshat I updated the one package that we were having an issue with, tests are running again locally (two failed tests, but at least they make sense).
2021-10-25 29824, 2021
akshat
That's great monkey!!
2021-10-25 29834, 2021
akshat
Let me have a look
2021-10-25 29844, 2021
monkey
The failing tests are probably due to the test harness not behaving well with canvas elements
2021-10-25 29845, 2021
monkey
I'll take that on, it's coming from the colourwheel library I incorporated
2021-10-25 29853, 2021
akshat
Ah ohkayy
2021-10-25 29841, 2021
lucifer
ruaok: should we discuss the inc dump stuff further?
2021-10-25 29831, 2021
ruaok
sure. I might drop off periodically as I am dealing with some BS customer support stuff for the office net connection.
2021-10-25 29855, 2021
lucifer
cool 👍
2021-10-25 29813, 2021
ruaok
fundamentally, I think we need to write the unique listens to a file and then ship that to spark
2021-10-25 29826, 2021
lucifer
so we intend to add a conusmer to unique queue, make it write to a temp dir and then publish that as the inc dump.
2021-10-25 29829, 2021
lucifer
right
2021-10-25 29836, 2021
ruaok
yes
2021-10-25 29852, 2021
ruaok
and we can write them in spark format as we go
2021-10-25 29835, 2021
lucifer
yes that sounds good to me.
2021-10-25 29833, 2021
lucifer
how important do you think this work is compared to listen counts in pg or say other LB tasks that may come up in near future (like we intended to start work on spotify roundup stuff next week) ?
2021-10-25 29850, 2021
ruaok
I think this is more important than listen counts (and should be quicker too).
2021-10-25 29811, 2021
ruaok
given that this is a public facing issue, I think it should get a reasonably high priority.
2021-10-25 29821, 2021
ruaok
since people are coming wishing to get stats.
2021-10-25 29803, 2021
Etua has quit
2021-10-25 29841, 2021
lucifer
agreed. i'll get on this next then. moving on to some implementation details.
2021-10-25 29831, 2021
ruaok
sweet
2021-10-25 29833, 2021
lucifer
1) should this consumer run in a new container or a part of existing ones?
2021-10-25 29816, 2021
lucifer
2) do we write every listen to the file and save it? (sounds inefficient but i may be wrong)
2021-10-25 29858, 2021
ruaok
1) I think it might be good to make a new container part of the mbid_mapping side of things.
2021-10-25 29814, 2021
ruaok
2) yes, that is fine. filesystems are fast and that isn't a problem.
2021-10-25 29854, 2021
lucifer
great., thanks. i'll get started on this soon and ask anything else that comes on the way when it comes.
nbin: I tested “GitHub OAuth for signing in to Jira” but couldn’t reproduce your issue. I enabled debug logs for this add-on so I can look into it if that happens again.
2021-10-25 29833, 2021
ruaok
lucifer: actually, I think we should add the output of the incremental listens to the timescale_writer. we already have the container, we already consume the listens. it would be trivial to write them to disk.
2021-10-25 29843, 2021
ruaok
I think I can do that pretty quickly. want me to go for it?
Will look to close things now that we're all kinda done
2021-10-25 29804, 2021
monkey
👍
2021-10-25 29844, 2021
TOPIC: MetaBrainz Community and Development channel | MusicBrainz non-development: #musicbrainz | BookBrainz: #bookbrainz | Channel is logged; see https://musicbrainz.org/doc/IRC for details | Agenda: Reviews, Follow up on Summit Notes -> Tickets, Allow test editing without verification (Freso/reo), Allow account admins to see editor-editor subscriptions? (Freso), removing empty accounts - last
2021-10-25 29812, 2021
ruaok
lucifer: ok, I think I worked out all the issues that pertain to new improved spark incremental dumps. lets see if I can knock it out tomorrow.
2021-10-25 29803, 2021
akshat
ruaok there are some python tests failing so if I could use a hand with that it'll be great
2021-10-25 29824, 2021
ruaok catches the ball and runs with it
2021-10-25 29845, 2021
ruaok
whenever you see failed test that has the error message "This connection has been closed", just re-run the tests. this is some heisenbug in our tests and we've not been able to sort out since it almost never happens on our dev machines.
2021-10-25 29805, 2021
ruaok
I've requested to re-run the tests. see what happens this time.
2021-10-25 29805, 2021
akshat
Oh okaay!
2021-10-25 29810, 2021
lucifer
ruaok: 👍
2021-10-25 29859, 2021
ruaok
wooo hooo! "All checks have passed"
2021-10-25 29805, 2021
ruaok
thanks monkey, akshat!
2021-10-25 29820, 2021
ruaok
now we just have to get that pile of code reviewed. :D
