Yeah. We can even ask in the forums for a list of URLs that can be of multiple types
2021-05-26 14637, 2021
reosarevok
So that we have a general idea which ones are there :)
2021-05-26 14646, 2021
reosarevok
re: yvanzo's comment earlier
2021-05-26 14624, 2021
reosarevok
yyoung: I'd suggest if we want to make this change and merge it, then it should actually support using the list :)
2021-05-26 14643, 2021
reosarevok
So, for non-auto-selected things, allow adding and selecting more types
2021-05-26 14651, 2021
yvanzo
the list?
2021-05-26 14609, 2021
reosarevok
Well, the idea is to "display relationships as a list."
2021-05-26 14618, 2021
reosarevok
But right now it's not a list, it's just one underneath, right?
2021-05-26 14618, 2021
yvanzo
that is related to using the relationship editor, right?
2021-05-26 14622, 2021
texke has quit
2021-05-26 14625, 2021
reosarevok
You still can't select more than one type
2021-05-26 14634, 2021
reosarevok
(for that you can only add a new link)
2021-05-26 14642, 2021
reosarevok
(well, add the same link again)
2021-05-26 14632, 2021
yvanzo
reosarevok: does it seem reasonnable to add support for errors target already?
2021-05-26 14628, 2021
reosarevok
yvanzo: IMO yes, if it's not too hard. But I'd still prioritize adding multiple reltypes to one link + blocking adding the same link twice.
2021-05-26 14630, 2021
yvanzo
so that we can postpone the UI stuff to have bitmap's input about using relationship editor instead of reinventing the wheel.
2021-05-26 14635, 2021
reosarevok
Oh, ok, in that sense
2021-05-26 14637, 2021
reosarevok
Sure, why not
2021-05-26 14610, 2021
yyoung
Also bitmap raises a concern that the new UI takes more space
2021-05-26 14635, 2021
yvanzo
yes and he is correct but that was not intended to be the final UI
2021-05-26 14601, 2021
reosarevok
It does, yes. One alternative to that to take less space would be to still have the type as the label if there's only one, but move it underneath if there's more than one. But that might be confusing
2021-05-26 14637, 2021
yvanzo
that would be confusing indeed
2021-05-26 14646, 2021
yyoung
yvanzo: How can we solve this problem with relationship editor?
2021-05-26 14626, 2021
yvanzo
yyoung: let's wait for having bitmap in the loop for this topic, it is the middle of his night right now.
2021-05-26 14647, 2021
reosarevok
Mind, I'm ok with it taking a bit more space :) I didn't think it was a big issue, although I guess it might be more so for very large entities with a ton of URLs
2021-05-26 14655, 2021
reosarevok
Yeah, let's get some more bitmap feedback
2021-05-26 14617, 2021
yyoung
yvanzo: Yes, but he suggests posting a thread on the forum to get more feedbacks
2021-05-26 14609, 2021
reosarevok
That also wouldn't hurt, especially if we throw the code up on test.mb
2021-05-26 14613, 2021
reosarevok
So that people can actually see it
2021-05-26 14611, 2021
yvanzo
reosarevok: the main point of this PR was to group rels by URL, not to optimize UI in any other way. For example, if errors are better handled, we can hide rel type selector if error is about the URL itself. There are a lot of possible optimizations.
2021-05-26 14617, 2021
yvanzo
yyoung: We seem to all agree that errors need better handling, that is a prerequisite improvement that can be done without changing the UI.
2021-05-26 14633, 2021
reosarevok
yvanzo: I get that :) It doesn't yet do that though, right? Since it still has only one rel per URL
2021-05-26 14653, 2021
yvanzo
yyoung: Also documenting the most popular use cases about relationship types combinations would be nice.
2021-05-26 14625, 2021
yvanzo
reosarevok: yes, it doesn't, just rearranged layout to allow that later on. I didn't see it would cause a regression.
2021-05-26 14645, 2021
reosarevok
Ok :)
2021-05-26 14624, 2021
yyoung
Before the UI change this disadvantage of error handling was not that obviously, though
2021-05-26 14619, 2021
reosarevok
I personally don't mind the layout change (other than the label index which is still weird to me) but before merging the change into beta/prod I'd want it to actually support multiple link types, since otherwise the community might be confused about the reasons :)
2021-05-26 14640, 2021
reosarevok
We could always have a branch we merge PRs against though, and then merge that into master when something is feature completeish?
2021-05-26 14601, 2021
yvanzo
yyoung: yes, your PR helped with finding more issues we didn't anticipate earlier :)
2021-05-26 14652, 2021
reosarevok
As it always happens :D
2021-05-26 14656, 2021
reosarevok
Please don't be discouraged :)
2021-05-26 14606, 2021
yvanzo
I switched it to draft status so that is more clear we are not willing to merge it as-is.
2021-05-26 14626, 2021
atj
The dropdown list is so irritating to use. Just restricting the entries to those detected as "permitted" would be a big usability gain IMV.
2021-05-26 14658, 2021
atj
Please ping me when you have something on beta, I use the URL links a lot so would be interested in the changes and happy to provide feedback.
2021-05-26 14634, 2021
reosarevok
We might have it on test before beta :) But we'll let people know if we do
2021-05-26 14642, 2021
jasondk
Hi, _lucifer: i had a question about CB. in the reviews table the 'source' table either contains 'BBC' if the review was imported, or it is null otherwise correct?
2021-05-26 14650, 2021
yyoung
yvanzo: OK, so is there anything else I need to do on this PR?
2021-05-26 14607, 2021
alastairp
jasondk: let me check for you
2021-05-26 14624, 2021
jasondk
thanks!
2021-05-26 14625, 2021
yyoung
atj: Agreed, and that would be great with your help :)
2021-05-26 14659, 2021
atj
I do think it will be really difficult to get right by the way - it's a complex problem to map
2021-05-26 14631, 2021
atj
My view would be that you make the 95% of use cases easier and try not to make the other 5% harder :)
But since, say, otherdatabases are lumped, then it's not too useful for that - plus there's no way to see "how many bandcamp links are free streaming vs free download vs purchase"
2021-05-26 14644, 2021
reosarevok
Still a lot better than nothing tho
2021-05-26 14603, 2021
reosarevok
I think we have a general idea where we stand, right?
2021-05-26 14608, 2021
reosarevok will go get breakfast if so :)
2021-05-26 14611, 2021
jasondk
Great. for my gsoc project i was planning on creating a new enum type to alter that column to accept 'BBC' or 'listenbrainz' or null as valid values
2021-05-26 14639, 2021
yvanzo
I did such statistics by host name in the past, I will try to find it back.
2021-05-26 14642, 2021
alastairp
jasondk: the BBC reviews are a special case, you can just ignore this column and always set it to null
2021-05-26 14652, 2021
yyoung
yvanzo: Sure. But the next task on the schedule is a bubble to display cleaned-up URL, I think that wouldn't introduce other issues?
2021-05-26 14629, 2021
alastairp
mmm, although - now that I think about it, that's an interesting idea. so the idea is that we'd be able to identify which reviews were written on critiquebrainz and which ones were written on listenbrainz?
2021-05-26 14634, 2021
atj
is that page showing that only 10% of artists have 1 or more URLs?
2021-05-26 14645, 2021
yvanzo
yyoung: this might change with redesign discussion going on :)
2021-05-26 14601, 2021
alastairp
is the source of the review the interesting data, or is the type of review the interesting data (e.g. long or short review)
2021-05-26 14615, 2021
yyoung
yvanzo: Alright :)
2021-05-26 14615, 2021
jasondk
alastairp: Yes, and then it would be indicated on the site in the future because I imagine reviews coming in from LB and CB would look different
2021-05-26 14628, 2021
jasondk
So i was just thinking to use the source column already existing
2021-05-26 14640, 2021
alastairp
let me read your proposal again
2021-05-26 14659, 2021
yvanzo
atj: good question, I'm not sure what 100% means for “has a discography page at” either
2021-05-26 14602, 2021
alastairp
jasondk: hmm, I think a new `review_type` column with values short/long would make a bit more sense. It's always a good idea to be explicit rather than try to reuse an existing feature
2021-05-26 14642, 2021
jasondk
now that I remember reviews coming from LB wont have a character cap but iliekcomputers mentioned that it would be a good idea to distinguish between lb and cb
2021-05-26 14651, 2021
alastairp
right. I would come up with a decision on what the difference is between the reviews. If it's short/long then we should use that as the distinguishing feature. If the only difference is the website that it comes from then maybe we don't need to track that
2021-05-26 14606, 2021
jasondk
alastairp: That makes sense. we would have to decide on how many characters a short vs long review is
2021-05-26 14634, 2021
reosarevok
atj: IIRC that means "of all relationships, 10% are artist-url ones
2021-05-26 14649, 2021
jasondk
Yeah, and then we could look through the existing reviews and sort them.
2021-05-26 14652, 2021
reosarevok
yvanzo: a bug, that's what that means - it seems to be the stats for "discography" above
2021-05-26 14650, 2021
reosarevok
I'll try to look into it after breakfast
2021-05-26 14657, 2021
atj
ah yes, I see it's 10% of all relationships
2021-05-26 14656, 2021
atj
was there a page/tag for artists with MB accounts?
2021-05-26 14616, 2021
atj
Just had the artist correct one of my submissions :)
alastairp: thanks! i'll read it carefully. could you please also add a section on how git2consul fits in here?
2021-05-26 14618, 2021
lucifer
my understanding is that it takes the json file from docker-server-configs directory and submits those to consul agent. but i am not sure if that is correct and if it is how that works or any gotchas to keep in mind.
2021-05-26 14652, 2021
alastairp
I'm not sure how much detail we should go into with this repo, if it should be completely self-contained or not
2021-05-26 14608, 2021
alastairp
or if we should focus on the documentation instead
2021-05-26 14614, 2021
samthursfield joined the channel
2021-05-26 14633, 2021
alastairp
yes, I wondered about that. keep in mind that I'm trying to document the app side of things, not the config/server side of things
2021-05-26 14653, 2021
lucifer
i think its fine to keep the repo for only stuff that we have to handle in a project itself. remaining parts of the setup can live somewhere else and maybe just add a link here.
2021-05-26 14659, 2021
lucifer
right makes sense.
2021-05-26 14622, 2021
alastairp
but also - we have this ongoing plan to replace some of the consul server parts, so we shouldn't go into that in too much more detail
2021-05-26 14655, 2021
lucifer
oh ok, sure that can be left for now then.
2021-05-26 14644, 2021
captainepoch has quit
2021-05-26 14644, 2021
captainepoch joined the channel
2021-05-26 14658, 2021
alastairp
lucifer: btw, did dumps work last night? (with the dumps directory not working)
2021-05-26 14637, 2021
lucifer
alastairp: nope no dump last night, also storage box was still down when i checked this morning.
2021-05-26 14619, 2021
alastairp
did we try and just unmount/remount it?
2021-05-26 14631, 2021
lucifer
sure, let's try that
2021-05-26 14625, 2021
lucifer
umount /mnt/dumps and then mount.cifs from history right?
2021-05-26 14651, 2021
alastairp
yes
2021-05-26 14621, 2021
lucifer
seems to be working now
2021-05-26 14635, 2021
lucifer
ls /mnt/dumps displays contents now
2021-05-26 14651, 2021
lucifer
i'll trigger an incremental dump then
2021-05-26 14608, 2021
alastairp
maybe restart cron too
2021-05-26 14613, 2021
alastairp
just to be sure it has the right mount
2021-05-26 14621, 2021
lucifer
makes sense, doing that first
2021-05-26 14632, 2021
lucifer
dump is now up on ftp.
2021-05-26 14654, 2021
lucifer
ruaok: alastairp: the 504s I mentioned earlier are related to labs api, the artist map endpoint calls labs api internally. labs api is 504ing on some requests, we probably do not handle that correctly so the artist map endpoint also returns a 504 to frontend.
