My preferable one is using named pipes (tho it would require separate modules for unix and windows)
2022-03-15 07408, 2022
skelly37
So I was also thinking about finding some free ports and communicate through them
2022-03-15 07422, 2022
outsidecontext
I think named pipes is also what most approaches I have seen are using
2022-03-15 07402, 2022
outsidecontext
the important part is indeed that we need something that works reliable across the different platforms
2022-03-15 07423, 2022
skelly37
For unix it's just few lines of code. Windows is more complicated with its win32pipe but I also managed to create simple scripts communicating with one another
2022-03-15 07413, 2022
outsidecontext
I personally never did use named pipes on Windows. but it already sounds good if you have experience on both platforms
2022-03-15 07436, 2022
PrathameshG
hey alastairp I checked out the site again today, but looks like MLHD hosting is still down. If you've found any mirrors so far, it would be really appreciated :)
2022-03-15 07450, 2022
outsidecontext
the basic setup might actually be not that much, but I assume there are quite a few edge cases to consider and proper testing
yes, we actually already utilize pywin32 for a few things, so that fits
2022-03-15 07428, 2022
zas
what about using multiprocessing module?
2022-03-15 07429, 2022
skelly37
To spawn a process reading the pipe?
2022-03-15 07446, 2022
outsidecontext
zas: the issue we need to solve is that a newly launched picard process needs to find and communicate with an already existing picard process. I don't think the multiprocessing module offers a ready solution for this
2022-03-15 07422, 2022
zas
outsidecontext: yes, I thought it was doing, but I just checked, and nope :(
2022-03-15 07450, 2022
riksucks
hi lucifer, so I was going to implement the backend of the hiding feature. I was wondering how the schema should be for it. We will be implementing the hiding feature only for listen and follow events. I was wondering how should I go about storing listens since they don't have any unique ID. Should I store em with 1) who listened 2) whose feed from which we want to hide 3) timestamp of the listen?
2022-03-15 07458, 2022
riksucks
monkey: maybe you would like to add some ideas for what I mentioned above
but not sure what the pros and cons compared to the pipe solution are
2022-03-15 07454, 2022
skelly37
When it comes to finding out whether Picard is already running, I am thinking about psutil.process_iter() (and looking for Picard[.exe] there). The advantage of such solution is that we have a decent wrapper of system utils, without using any ifs to check out the platform. Works for Windows, Mac, Linux (and Solaris, BSD if someone was to care about it here hah)
2022-03-15 07400, 2022
zas
it would add another dependency to Qt possible bugs in this field ... ;)
2022-03-15 07426, 2022
outsidecontext
yep, happy to use a qt independent solution as well
2022-03-15 07432, 2022
mayhem
Postgres, the Rick Astley of databases. :)
2022-03-15 07450, 2022
riksucks
and also, we have to store follow events in the same database. So maybe we can have following columns
2022-03-15 07450, 2022
riksucks
1) user who generated event 2) which user's feed is it? 3) event_type 4) track_name 5) timestamp (timestamp to specifically identify different listens)
2022-03-15 07415, 2022
outsidecontext
skelly37: would we need this if looking for the proper named pipe is needed anyway because the list of file paths needs to be passed to the existing process?
2022-03-15 07459, 2022
skelly37
After looking for picard process, the launcher would either spawn a new window, recreate the pipe (to avoid errors) or would send argv to the existing pipe and exit(0)
2022-03-15 07421, 2022
zas
I don't think we need psutil.process_iter() if we use named pipes
2022-03-15 07435, 2022
monkey
riksucks: The way I understood last time, we were ignoring listens on the feed page (i.e. e can't hide them)
2022-03-15 07456, 2022
skelly37
outsidecontext: I agree on that adding Qt-based solution would just complicate the work
2022-03-15 07408, 2022
outsidecontext
what we should consider in any way that the same instance is only used if the same executable was being run, so different pipes for different executable paths I think
2022-03-15 07416, 2022
monkey
We can 1. hide events from the feed (but not listens) and 2. mark listens as to-be-deleted on the user listens page (not the feed)
2022-03-15 07429, 2022
monkey
1. should be pretty easy since we have even ids, so we can store, as you say, [id of current user, event id]
2022-03-15 07408, 2022
skelly37
zas, outsidecontext: Why different executable paths? I was thinking that we were to enforce one and only one instance mode.
2022-03-15 07423, 2022
zas
basically process 1 check if the pipe exists (if not create it), and ask though the pipe if it's a first or a Nth instance, pipes can be named after the picard version (or any checksum)
2022-03-15 07425, 2022
outsidecontext
skelly37: I usually have multiple versions / variants of picard installed, it's odd if I launch Picard 2.x, but files open in the already running Picard 2.y
2022-03-15 07441, 2022
reosarevok
bitmap: several builds were up before beta, but finally putting it out now
2022-03-15 07448, 2022
zas
outsidecontext: I guess we need something like 'communication protocol version' for those
2022-03-15 07407, 2022
lucifer
monkey: riksucks: which all events do we want to be hidable?
2022-03-15 07417, 2022
skelly37
So my idea is to hardcode path like /tmp/picard-v-$version_name
2022-03-15 07423, 2022
riksucks
monkey: ohh my bad. yes. Since follows aren't stored in the event database and are just merely retrieved, we can just store [user_0, user_1]
2022-03-15 07429, 2022
riksucks
lucifer: only follows
2022-03-15 07438, 2022
monkey
lucifer: events on the feed page from other users
2022-03-15 07452, 2022
monkey
riksucks: only follows? that's not what I remember either :p
2022-03-15 07458, 2022
outsidecontext
yes, something like this. also having a command line parameter to force creation of a new instance would be good (that would run without pipe than I assume) for the cases where users explicitly want a second instance
2022-03-15 07429, 2022
lucifer
"events on the feed page from other users" yeah that's what i remember so wanted to confirm :)
2022-03-15 07455, 2022
zas
I wonder if we should use picard versions or a protocol version for naming, it looks to me communicating between versions should be possible, if they are compatible
2022-03-15 07402, 2022
monkey
So in short, all feed events other than "XXX listened to a track"
2022-03-15 07436, 2022
lucifer
makes sense
2022-03-15 07442, 2022
zas
outsidecontext: new instance could have its own pipe, and we should be able to pass the pipe path by command line
2022-03-15 07410, 2022
lucifer
let me review how we generate/store the events once and i'll review your suggested solution then
2022-03-15 07437, 2022
monkey
Oh, and not the user's own events either, since the user can delete those themselves
2022-03-15 07459, 2022
outsidecontext
zas: I don't think so actually. What I want to solve is that I can launch the same executable multiple times and it passes the files to the running instance, but if I launch a different picard version installed elsewhere it launches separately. in that scenario they would never communicate with each other. but we can of course define a small message being exchanged to check for communication protocol to ensure
2022-03-15 07459, 2022
outsidecontext
compatibility
2022-03-15 07402, 2022
monkey
Although now that I say it out loud, I don't know if that should include events like "you are now following XXX". I can't delete that one, so should I be able to hide it?
2022-03-15 07438, 2022
skelly37
outsidecontext: Okay for me, but I think that forced new instances shouldn't have pipes. One default instance per version.
2022-03-15 07445, 2022
lucifer
hmm, maybe having filters would be better for that?
2022-03-15 07436, 2022
outsidecontext
skelly37: I tend to agree. but actually those are details I think we can finalize after an initial basic version is working. it seems to be mostly about defining how the pipe name gets constructed :)
2022-03-15 07453, 2022
zas
^^yup
2022-03-15 07446, 2022
bitmap
reosarevok: thanks, lmk when it's done and I can run the report
2022-03-15 07451, 2022
outsidecontext
seeing how this goes it likely could be that this is far less work than we originally feared. but if we have time left we can see if we can utilize this setup to optimize our crash error handling. currently we don't handle any crash errors for some circumstances.
2022-03-15 07420, 2022
skelly37
So, details (like naming issues, multiple versions, forced versions etc.) after making pipes work for one and only one instance?
2022-03-15 07436, 2022
outsidecontext
if we had the launcher process very simple and only handle the check if the pipe already exists and launch the actual picard as a separate process if necessary we might be able to better react on this process crashing and showing something useful to the user
2022-03-15 07411, 2022
outsidecontext
skelly37: I would actually say so, yes. with a tendency to do a pipe name on either actual executable path or version number
2022-03-15 07437, 2022
skelly37
outsidecontext: Great idea, agreed. (doing errors handling to fill the time after the launcher works brilliant)
2022-03-15 07405, 2022
zas
executable path would not work, if one upgrades picard while an instance is running, the path can be the same, the version of the second instance might differ
[bookbrainz-site] 14MonkeyDo merged pull request #794 (03master…dependabot/npm_and_yarn/compression-webpack-plugin-9.2.0): chore(deps-dev): bump compression-webpack-plugin from 8.0.1 to 9.2.0 https://github.com/metabrainz/bookbrainz-site/pul…
2022-03-15 07458, 2022
outsidecontext
I think the proposal already should define some proposed behavior there, even if we might decide together to change details later on.
2022-03-15 07437, 2022
BrainzGit
[bookbrainz-site] 14MonkeyDo merged pull request #796 (03master…dependabot/npm_and_yarn/babel/plugin-transform-runtime-7.17.0): chore(deps-dev): bump @babel/plugin-transform-runtime from 7.16.4 to 7.17.0 https://github.com/metabrainz/bookbrainz-site/pul…
2022-03-15 07440, 2022
skelly37
rdswift: especially because I'm strongly for making one default instance per Picard version. Other ones should be only --force ones. Otherwise the selection of the instance the launcher should communicate with would become unnecessarily complicated imo.
2022-03-15 07459, 2022
outsidecontext
+1
2022-03-15 07412, 2022
rdswift
👍
2022-03-15 07424, 2022
BrainzGit
[bookbrainz-site] 14dependabot[bot] opened pull request #809 (03master…dependabot/npm_and_yarn/react-bootstrap-2.2.1): chore(deps): bump react-bootstrap from 1.6.4 to 2.2.1 https://github.com/metabrainz/bookbrainz-site/pul…
2022-03-15 07447, 2022
BrainzGit
[bookbrainz-site] 14dependabot[bot] opened pull request #810 (03master…dependabot/npm_and_yarn/prop-types-15.8.1): chore(deps): bump prop-types from 15.7.2 to 15.8.1 https://github.com/metabrainz/bookbrainz-site/pul…
2022-03-15 07409, 2022
skelly37
zas, outsidecontext: So, I start working on the application now and in few days I reach you for review?
2022-03-15 07437, 2022
zas
skelly37: yes, perfect, we are always more or less around, just ping on this channel if you have questions
2022-03-15 07453, 2022
outsidecontext
skelly37: yes, that would be perfect. please put your proposal on https://community.metabrainz.org/ for discussion. that might mean also others might comment on it, which might be interesting. official submission of the final application to gsoc then will be starting April 4th
2022-03-15 07425, 2022
PrathameshG has quit
2022-03-15 07441, 2022
skelly37
rdswift: To put it simple: One default instance per picard version to send argv there, additional ones should be used with --force flag or sth like that. The forced ones wouldn't operate with pipes not to overcomplicate the logic. That's why /tmp/picard-pipe-v2-7-0, for example, could do imo
2022-03-15 07400, 2022
skelly37
Great, thanks for the help already then :>
2022-03-15 07403, 2022
outsidecontext
I'm also around here and definitely will read anything that mentions my name or picard :D
2022-03-15 07423, 2022
outsidecontext
ok, thanks so far. I'm off for dinner
2022-03-15 07412, 2022
zas
enjoy
2022-03-15 07435, 2022
skelly37
I have also a question related to our workflow. Flexible hours as long as the work is done are okay or do you have by any chance a more strict policy (like some organizations I've seen on gsoc)?
2022-03-15 07440, 2022
skelly37
enjoy btw
2022-03-15 07447, 2022
PrathameshG joined the channel
2022-03-15 07411, 2022
reosarevok
We work flexible hours ourselves, so I'd be surprised we'd ask for anything else from gsoc, if the work gets done
2022-03-15 07418, 2022
reosarevok
(but I guess that's up to each mentor :) )
2022-03-15 07455, 2022
riksucks
hi lucifer, monkey, seems like I had left halfway in the middle of the previous convo which is why I don't remember all these 😅. So let me recap on what I have to do. I have to hide any event that is being generated by someone else (apart from listens)
2022-03-15 07420, 2022
reosarevok
aerozol: it seems I had done a bit of a dumb when releasing yesterday. I *think* the importer preview issue will be fixed later today
2022-03-15 07421, 2022
riksucks
monkey: can we implement greyed out trash icon for listens, for uniformity? or do you have something else in mind
2022-03-15 07448, 2022
monkey
I didn't think about that. I'd rather not have the icon for now, it suggests this is a possible thing for users to do bu somehow they're not allowed or able to.
2022-03-15 07416, 2022
monkey
And we might end up removing those listens from the feed in the future, so it's not the top worry as far as I'm concerned
[bookbrainz-site] 14dependabot[bot] opened pull request #811 (03master…dependabot/npm_and_yarn/css-loader-6.7.1): chore(deps-dev): bump css-loader from 6.5.1 to 6.7.1 https://github.com/metabrainz/bookbrainz-site/pul…
2022-03-15 07439, 2022
BrainzGit
[bookbrainz-site] 14dependabot[bot] opened pull request #812 (03master…dependabot/npm_and_yarn/influx-5.9.3): chore(deps): bump influx from 5.9.2 to 5.9.3 https://github.com/metabrainz/bookbrainz-site/pul…
2022-03-15 07448, 2022
monkey
Damn youuuuu dependabot !
2022-03-15 07447, 2022
Ansh
monkey: For the BB-CB project, we discussed lowering the project length to 175 hours. Are there specific tasks you would like to get done during the period ?
2022-03-15 07435, 2022
monkey
Hi Ansh ! I did discuss the project with alastairp, and looking at how quick you've been pumping out PRs for CB, we both feel 175 hours sounds about right for the whole project
2022-03-15 07443, 2022
monkey
The priority would be to do everything on the CritiqueBrainz side for starters, meaning enabling reviews for BB entities.
2022-03-15 07426, 2022
monkey
And only after that is done should we look at implementing the BookBrainz side of things (which I also don't expect to take a very long time)
2022-03-15 07428, 2022
Ansh
Yes that can be easily managed.
2022-03-15 07424, 2022
Ansh
I would require time for the bookbrainz frontend side.
2022-03-15 07440, 2022
monkey
Understandable. If time becomes too constrained I'll be hopping in to solve some of the UI aspects (like do mockups and styling)
lucifer: yeah, CB _expects_ there to be a type returned with objects, but it doesn't request one
2022-03-15 07401, 2022
lucifer
huh!
2022-03-15 07401, 2022
alastairp
this is what I've been finding, lots and lots of places in templates where it's just silently skipping missing data. I turned on the jinja setting to fail if a template statement doesn't resolve to anything, it's throwing up many situations like this
2022-03-15 07423, 2022
lucifer
also, a downside of code being split across BU/CB.
2022-03-15 07448, 2022
alastairp
I can see that this is all because of the CB data originally coming from pymbngs
2022-03-15 07422, 2022
alastairp
and when we moved to getting from the db, things weren't copied over perfectly, and because jinja doesn't care if something doesn't exist no one caught the problem
2022-03-15 07446, 2022
lucifer
i see. makes sense
2022-03-15 07456, 2022
alastairp
the build process is a little annoying to make a change in BU and then apply it in CB, but it's not terrible
2022-03-15 07414, 2022
alastairp
I think there's still value to keep this in BU for other projects
