i haven't deployed master to the production API yet, so the 404 makes sense
BrainzGit
[bookbrainz-site] prabalsingh24 opened pull request #435 (UserCollection…search-table-add-to-collection): UserCollection: Add to collections from search results https://github.com/bookbrainz/bookbrainz-site/p...
[listenbrainz-server] paramsingh merged pull request #884 (master…master): LB-610: The URL in the browser should update with parameters for the history page https://github.com/metabrainz/listenbrainz-serv...
Leftmost
Mr_Monkey, I'd say we publish -data-js for now. I think what I've done is sufficient to cover the work I'm doing.
Mr_Monkey
OK, I'll aim to do that today
Leftmost
👍
And anyways, version numbers are cheap. :)
ishaanshah
iliekcomputers: Please ping me when you are free, had to discuss some stuff about the request consumer code
But as that function is used for recommendation too, we cant change it
So I was thinking that we should have messenger_map similar to query_map
the messenger_map will map the query type to the appropriate messaging function
jmp_music has quit
iliekcomputers
hmm, let me think
i was looking into exploring making classes that inherit from an abstract RequestConsumerQuery for each query
ishaanshah
So each class would implement its own handler and messenger?
iliekcomputers
right, we could have a default messenger, and the class could override it if it needs to
reosarevok
yvanzo: I guess the main issue is the lack of parity
ishaanshah
Hmm, that will work too, but wont we have multiple rmq connections open?
shivam-kapila
ruaok: Ping
iliekcomputers
hmm, i haven't fleshed it out yet. i'll try to write it up in some more detail today.
ishaanshah
Ok, I will keep the get_artist and get_release ready
ruaok
pong!
shivam-kapila
Hey
The failing dump manager test is also fixed now
ruaok
what was it?
shivam-kapila
Remember the 0 listens dumped log I sent you?
That was the cause. No listens had created in the dump formation range
ruaok
ah, yes.
makes sense.
push your change, please.
shivam-kapila
Pushed. timescale-rebased-again
This shouldn't happen though as the previous dump id is added before inserting listens. Anyways I just made the previous dump ID time to be less by 5 sec.
Took 2.5 days to finally dig the issue :-|
ruaok
one thing I was thinking of is that we should change the timestamps in our test data, to include a gap of more than 3 weeks.
I'm glad you found it, it looked totally opaque to me.
shivam-kapila
> one thing I was thinking of is that we should change the timestamps in our test data, to include a gap of more than 3 weeks.
Why so?
iliekcomputers
ishaanshah: did you figure out the issue with CORS?
ishaanshah
Not yet, I am not sure why the server is not sending the header with 204 response
ruaok
shivam-kapila: that would force us to deal with the time-range issues correctly. I think having closely spaced listens may be hiding problems for us.
ishaanshah
Thats why the error boundary is not working
iliekcomputers
can you reproduce it in dev?
ishaanshah
It works on dev
shivam-kapila
ruaok: Hm. Yeah I will agree to that
iliekcomputers
yeah, it worked for me too.
i do wonder tho, why it works for the artist endpoint and not for the release endpoint
ruaok
shivam-kapila: can you please try it? add a few more listens that have a large gap and see what breaks?
ishaanshah
artist endpoint is on api
ruaok
integration tests pass!
!m shivam-kapila
BrainzBot
You're doing good work, shivam-kapila!
ishaanshah
release is on beta-api
maybe some config error on beta-api?
iliekcomputers
they're basically the same, but yeah, that could be a possibility
shivam-kapila
ruaok: Thanks.
> can you please try it? add a few more listens that have a large gap and see what breaks?
To the test JSON files right?
ruaok
yes
shivam-kapila
I will give that a try. If things break I guess they would be in timescale_listenstore tests only but lets see