#metabrainz

ok

My preferable one is using named pipes (tho it would require separate modules for unix and windows)

So I was also thinking about finding some free ports and communicate through them

I think named pipes is also what most approaches I have seen are using

the important part is indeed that we need something that works reliable across the different platforms

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

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 :)

the basic setup might actually be not that much, but I assume there are quite a few edge cases to consider and proper testing

I can upload you the "PoC" files somewhere to show you how it works on a base level (looped sending of i+=1 to the pipe)

Also I wrote it on a school PC, without admin or even UAC.

here's the module I'm talking about: https://pypi.org/project/pywin32/

yes, we actually already utilize pywin32 for a few things, so that fits

what about using multiprocessing module?

To spawn a process reading the pipe?

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

outsidecontext: yes, I thought it was doing, but I just checked, and nope :(

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?

monkey: maybe you would like to add some ideas for what I mentioned above

skelly37, zas: what I had also seen is using QSharedMemory https://doc.qt.io/qt-5/qsharedmemory.html

but not sure what the pros and cons compared to the pipe solution are

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)

it would add another dependency to Qt possible bugs in this field ... ;)

yep, happy to use a qt independent solution as well

Postgres, the Rick Astley of databases. :)

and also, we have to store follow events in the same database. So maybe we can have following columns

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)

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?

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)

I don't think we need psutil.process_iter() if we use named pipes

riksucks: The way I understood last time, we were ignoring listens on the feed page (i.e. e can't hide them)

outsidecontext: I agree on that adding Qt-based solution would just complicate the work

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

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)

1. should be pretty easy since we have even ids, so we can store, as you say, [id of current user, event id]

zas, outsidecontext: Why different executable paths? I was thinking that we were to enforce one and only one instance mode.

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)

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

bitmap: several builds were up before beta, but finally putting it out now

outsidecontext: I guess we need something like 'communication protocol version' for those

monkey: riksucks: which all events do we want to be hidable?

So my idea is to hardcode path like /tmp/picard-v-$version_name

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]

lucifer: only follows

lucifer: events on the feed page from other users

riksucks: only follows? that's not what I remember either :p

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

"events on the feed page from other users" yeah that's what i remember so wanted to confirm :)

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

So in short, all feed events other than "XXX listened to a track"

makes sense

outsidecontext: new instance could have its own pipe, and we should be able to pass the pipe path by command line

let me review how we generate/store the events once and i'll review your suggested solution then

Oh, and not the user's own events either, since the user can delete those themselves

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

compatibility

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?

outsidecontext: Okay for me, but I think that forced new instances shouldn't have pipes. One default instance per version.

hmm, maybe having filters would be better for that?

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 :)

^^yup

reosarevok: thanks, lmk when it's done and I can run the report

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.

So, details (like naming issues, multiple versions, forced versions etc.) after making pipes work for one and only one instance?

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

skelly37: I would actually say so, yes. with a tendency to do a pipe name on either actual executable path or version number

outsidecontext: Great idea, agreed. (doing errors handling to fill the time after the launcher works brilliant)

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

(on linux at least)

good point

[bookbrainz-site] 14MonkeyDo opened pull request #808 (03master…monkey-revision-note-links): feat(BB-564): Make revision notes links clickable https://github.com/metabrainz/bookbrainz-site/p...

I think we should use a fixed name, but ensure only compatible versions can communicate

Hash of the executable?

Yeah, I agree with zas. Fixed path with fixed version in the filename.

Hashes would be overkill imo

Yeah, probably.

[bookbrainz-site] 14MonkeyDo closed pull request #799 (03master…index-languages-for-search): feat(LanguageField): Index languages for search https://github.com/metabrainz/bookbrainz-site/p...

skelly37: so for doing this as part of gsoc we need a proposal and actual application for gsoc, see https://musicbrainz.org/doc/Development/Summer_...

[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/p...

I think the proposal already should define some proposed behavior there, even if we might decide together to change details later on.

[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/p...

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.

+1

👍

[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/p...

[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/p...

zas, outsidecontext: So, I start working on the application now and in few days I reach you for review?

skelly37: yes, perfect, we are always more or less around, just ping on this channel if you have questions

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

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

Great, thanks for the help already then :>

I'm also around here and definitely will read anything that mentions my name or picard :D

ok, thanks so far. I'm off for dinner

enjoy

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)?

enjoy btw

We work flexible hours ourselves, so I'd be surprised we'd ask for anything else from gsoc, if the work gets done

(but I guess that's up to each mentor :) )

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)

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

monkey: can we implement greyed out trash icon for listens, for uniformity? or do you have something else in mind

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.

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

[brainzutils-python] 14alastair opened pull request #81 (03master…musicbrainzdb_data): Improve data returned in musicbrainz_db module https://github.com/metabrainz/brainzutils-pytho...

lucifer: ^ I fixed some bugs in the BU mb module

used in CB

alastairp: the changes to default includes make sense to me. if we use type in CB then definitely return it by default otherwise fine either way.

thanks lucifer, i had the same thought

monkey: I see, so for the events that need to be hidden, I have to implement a minus sign logo. right?

I think that would work yes. Either that or an eye icon

reosarevok: and turn around and desert.. tokens

[bookbrainz-site] 14MonkeyDo merged pull request #743 (03master…sanitize-spaces): Fix[BB-602]: Sanitize spaces in editing workflow https://github.com/metabrainz/bookbrainz-site/p...

I'll switch from kiki to herb for few minutes, upgrading openresty

switching back to kiki

[bookbrainz-site] 14MonkeyDo merged pull request #797 (03master…dependabot/npm_and_yarn/webpack-dev-middleware-5.3.1): chore(deps-dev): bump webpack-dev-middleware from 5.3.0 to 5.3.1 https://github.com/metabrainz/bookbrainz-site/p...

[bookbrainz-site] 14MonkeyDo merged pull request #798 (03master…dependabot/npm_and_yarn/babel/cli-7.17.6): chore(deps-dev): bump @babel/cli from 7.16.0 to 7.17.6 https://github.com/metabrainz/bookbrainz-site/p...

[bookbrainz-site] 14MonkeyDo merged pull request #806 (03master…dependabot/npm_and_yarn/babel/runtime-7.17.7): chore(deps): bump @babel/runtime from 7.16.3 to 7.17.7 https://github.com/metabrainz/bookbrainz-site/p...

[bookbrainz-site] 14MonkeyDo merged pull request #802 (03master…www-regex): Fix(Annotation): Use word boundary in adding http prefix https://github.com/metabrainz/bookbrainz-site/p...

[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/p...

[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/p...

Damn youuuuu dependabot !

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 ?

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

The priority would be to do everything on the CritiqueBrainz side for starters, meaning enabling reviews for BB entities.

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)

Yes that can be easily managed.

I would require time for the bookbrainz frontend side.

Understandable. If time becomes too constrained I'll be hopping in to solve some of the UI aspects (like do mockups and styling)

[bookbrainz-site] 14MonkeyDo merged pull request #785 (03master…generate-isbns): Fix(BB-644): Auto append new isbn identifier row https://github.com/metabrainz/bookbrainz-site/p...

[bookbrainz-site] 14MonkeyDo closed 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/p...

[bookbrainz-site] 14MonkeyDo merged 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/p...

[bookbrainz-site] 14MonkeyDo merged pull request #807 (03master…editor-form): Fix(editor): Add comparision for genderId https://github.com/metabrainz/bookbrainz-site/p...

lucifer: yeah, CB _expects_ there to be a type returned with objects, but it doesn't request one

huh!

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

also, a downside of code being split across BU/CB.

I can see that this is all because of the CB data originally coming from pymbngs

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

i see. makes sense

the build process is a little annoying to make a change in BU and then apply it in CB, but it's not terrible

I think there's still value to keep this in BU for other projects

[listenbrainz-server] 14amCap1712 opened pull request #1903 (03master…ts-ts): Fix handling of latest listens in get_timestamps_for_user https://github.com/metabrainz/listenbrainz-serv...

true but AB is going and LB's MB needs mostly require hand written queries.

yeah, perhaps that's true

i guess Troi could use it so worth keeping for now.