riksucks: lucifer has guests this week, so might not be around as much. can I help?
reosarevok: hi, I'm trying to find the bit in the MB API which filters release group types - specifically the bit that checks if a given type is valid, and the bit which uses this provided filter to restrict the list of rgs. Are these values checked directly against the release_group_primary_type table, or are they hard-coded in the server?
riksucks
ohh I see, my bad. alastairp, I was also gonna ping you regarding one discussion. So I was working on a PR where I implemented hiding of user feed events in the backend. And in the code, I had implemented a Bad Request, if user tried to hide an already hidden event
do you think it should be that way? or do you think that we should return {"status": "ok"} if user tries to hide event that has already been hidden
riksucks: hmm, good question. this is similar to a few other places that we have in the API - such as our delete API, where I believe we don't notify the user if they try and delete something that doesn't already exist
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, Securing MeB infrastructure - part 3 | Telegram-bridge and #musicbrainz, catquest (freso/jwlory)
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, Securing MeB infrastructure - part 3 | Telegram-bridge and #musicbrainz, catquest (freso/jwflory)
riksucks
alastairp: I see, so I should remove the bad request when user tries to hide it twice, right? Apart from that, do you think the PR needs any more changes, or any other feedbacks that you might have in your mind? Thanks :)
alastairp
yes, at the moment I think we should remove bad request
in fact, I don't think it's quite the correct code according to the strict definition: https://datatracker.ietf.org/doc/html/rfc7231#s..., in my understanding repeating this query multiple times should always result in the same error (maybe this was a definition in an older version of the RFC)
technically, this query could fail some times and then succeed (because elsewhere someone un-hid the event)
412 Precondition Failed could be an option here, but I don't think that's important
riksucks
that makes sense
alastairp
one other option is that we could return HTTP 200, with status: ok, but also with a log message - saying either "event hidden" if it was hidden, or "event was already hidden" if it had already been done. However, can you think of any cases where a client would want to know the difference between if they hid the event, or someone else hid it?
> However, 400 is appropriate because "The request could not be understood by the server due to malformed syntax. The client SHOULD NOT repeat the request without modifications."
it's interesting. that last sentence appears to have been removed in the new version of the HTTP RFC
riksucks
you know I was thinking just the same, does the client need to know that the event was already hidden, and that we tried to hide it again, because these hiding/unhiding would be done by the same user across multiple devices.
only case I can think of is, suppose someone browses the feed from device A, and hides events in B, then he may go back to device A and wonder why the events aren't hidden
maybe we can send "The event was already hidden" message when the user tries to re-hide the same events from new device?
alastairp
yes, right. I think that this is probably quite an uncommon event, in which case I wouldn't worry too much about it. We also have a system for sending messages back to the client from the server (the websockets system, we use it to push new listens to a user's listens page). So a better solution to this problem would probably be to add events to the websockets system
however, my recommendation would be to not worry about this for now - just implement your current PR without any extra checks, and then we could add a new feature to add the websockets to the feeed page
riksucks
I see, makes sense, so a new feature can be added to JIRA for real time feed, with both hiding and deleting happening in real time
alastairp
yes, perfect
I'll open your PR and put it on my todo list to review, though if lucifer has already looked at it then that might be enough.
riksucks
I see thanks :), I will create the JIRA ticket and update the bad request bit rn.
tangentially related is a philosophy I've been slowly cultivating with my data processing containers... the idea of not keeping explicit state of a process, but have the state of the process be discoverable by the process when it gets restarted.
"If you’re using the web interface or web service, run ./admin/BuildMaterializedTables --database=MAINTENANCE all to build new materialized tables. These will take several additional gigabytes of spaces and be kept up-to-date automatically via triggers. For more information, see INSTALL.md."
alastairp
I see it
interesting - the release notes for musicbrainz-docker explains this too - https://github.com/metabrainz/musicbrainz-docke..., but our mirror was set up only a few months ago. so I wonder why this table isn't populated, maybe it's not set up by default? I'll double-check
q3lont: I just saw your mb-docker issue (assuming it's you since it's the same topic) - I'm not sure it's doable, others might be better at answering that, but
Unless you are not using release / release group / track artists at all, you'd certainly need to keep artist_credit at the very least
The other tables do not store an artist name
If you want anything else than an artist string you'd also need more tables, but :)
alastairp: the order is slightly different with slow vs fast
(fast sorts by unofficial first)
But yeah
q3lont
reosarevok, yeah I need artist_credit too
reosarevok
If you want release events you'll need release_country and area, etc