#metabrainz

https://usercontent.irccloud-cdn.com/file/wTQn5... looks *great* (would use a orange instead of purple on the ":(" face is only) - the unnecessary 👍 icon and I think we're good to go!

here is a preview of what reosarevok's mini dress is going to look like. cool, no?

Looks neat!

is is really cool. :)

Cute

[bookbrainz-site] MonkeyDo closed pull request #526 (master…uifixes): Changed the styling of buttons to enhance the semanticity of website https://github.com/bookbrainz/bookbrainz-site/p...

Yay, pink death metal!

[musicbrainz-server] reosarevok opened pull request #1765 (master…MBS-11204): MBS-11204: Avoid crashing if entity1_id is missing https://github.com/metabrainz/musicbrainz-serve...

MBS-11204: ISE: Validation failed for \'Int\' with value undef https://tickets.metabrainz.org/browse/MBS-11204

ruaok: send you the CSR for apple

distribution is what we have now, so that might be right.

let me upload this and see if it works.

The pink tee looks great!

ruaok: I tried it, as I also thought we are using the distribution one now. Signing works of course, but Apple's notarization service complains then about this not being a valid certificate. Docs also say you should use the Developer ID certificate for distribution outside of app store. Not sure, maybe they changed something.

outsidecontext: You seem to have created a new dist cert with expiration 2021/10/28.

yes, that's the one I tried

I'm not sure what to do next....

oh yes, got stuck there, too. the + sign in the blue circle

yes, I found that yesterday, but we have developer ID applications that don't expire for 2-3 years. do we need to create a new one?

ruaok: nope, I checked again. All good it seems. We use the one that expires in 2023-07-27

ok, ping me if something goes off the rails.

will do, thanks

zas: reosarevok : yvanzo : MB seems to be flapping up and down. any ideas?

Nothing we have changed recently, I'd expect - beta worked just fine. Seems to be back to normal now?

yea

I'd kinda expect it's some extra load for reasons, but if not we can look into it

(as in, if zas' graph shows nothing weird incoming, maybe we dun goofed)

it keeps flapping: reosarevok can you please restart the mb website container on 10.2.2.29?

[troi-recommendation-playground] alastair opened pull request #22 (main…element-debug): command-line interface to debug a single element https://github.com/metabrainz/troi-recommendati...

ruaok: done, I think

So far,vso good. Thx.

ruaok: hi, feedback on #22 would be great - I'm still not sure if it's a useful idea. I did actually make it in response to a specific piece of debugging that I was trying to do, so it does work in this specific use-case

Ok, will look in a minute.

I found out earlier that recording year lookup is quite buggy.

[troi-recommendation-playground] alastair opened pull request #23 (main…requests-params): Use request's params argument instead of encoding manually https://github.com/metabrainz/troi-recommendati...

meh. can't think clearly today.

there's this imposing sense of dread sitting over everything this week

literally everything.

but me sleeping poorly is my main issue -- I've gotten used to the feeling of dread. #2020

I hope that improves for you

thx. should be ok. I think that was a fluke.

hmm

now I am really confused about super and when the base class __init__ need to be called.

yeah, I was just going to ask about that

it needs to be called for all Elements, but no args need to be passed.

correct

but unless I explicitly call it, the object does not correctly get initialized.

so, I really need all those calls to super.

because python doesn't implicitly call parent class methods

Element does something in its init, so every time you explicitly make an init for a subclass (because you want to take in an argument), you must super().__init__()

your statement of "this init isn't needed if it only calls super, can be removed. There are a few items like this, both with no arguments and some subclasses of Patch that pass debug" isn't quite correct then.

mmm, I meant things of the style:

which is a form, i really prefer, tbh. just seems cleaner.

I'm just going to back out all my commits since I started working on improvemetns and start over again.

right, so I guess it's making it explicit that this class takes no arguments in its constructor

Ok, #21 updated.

#22 looks interesting. how do you test when you need to pass an array of Recordings ?

that's a matter of how you decide to implement generate_debug_input - the input is an array, so you could loop through all args and return x Recordings

your comment on the PR is right too, if you need lots of data in the input object, then you might need to pass in many many args

I see this getting very cumbersome.

I almost thought about implementing a full commandline parser that lets you string together elements, but that's getting crazy

yeah, it definitely has that posibility, which is why I put it up as-is to get some discussion going

would it be better to have a DebugRecordingElement that can take a recording MBID, look things up and then pass it in? Maybe with support for constructing debugging pipelines from the command line?

I don't think I want to construct pipelines from the commandline, it sounds like the parsing for that will get complex really quickly

hmm. I can't really see a way of making this a whole lot easlier without a whole lot of work.

agreed.

right now I can see the data that needs to be passed into an Element be pretty simple and this the debugging is simple.

*thus

but later on we'll have more complex inputs that will be required to test Elements and I see a command line interface just not cutting it.

but even take something as simple as the ArtistCreditLimiterElement

you need to pass in 2 args to the constructor, and then each item needs a bunch of stuff, in both Recording and Recording.artist

maybe by this stage it's easier to just write a unittest

as I said, the main motivation was to get out of the habit of loading ipython, trying something, making a change, reloading ipython to get the change, and try again

your motivation is that testing an element with real data is too much data flowing for easy debugging. is that right?

right. in this case I wanted to test the metadata loader, but I was using it in the pipeline with AB similarity, so it was passing in 160 Recordings when I just wanted 1

so, what if we hone in on this one specific feature then?

something like a "limited data debugging" mode?

ohh, interesting

construct pipeline, run pipeline, but terminate once you get 1 data element passed into the target element?

yeah. optionally, an element could continue to have the `debug_print_response` method that I added, where it can print additional debug information before quitting

so even if a pipeline had 5 elements, you could tell it to stop after the 3rd one?

I suppose.

this was my specific usecase - I had the RecordingLookupElement and then a filter, but the recording lookup was failing. If I debug a pipeline and go all the way through to the last element it's "too late", as it were

So would that be a halfpipe?

🛹

I think we can accomplish this by specifiying an element under test and then have the generate() function recognize when its about to pass data to that element, truncate all the data, but one, pass it in and when read() returns, terminate.

reosarevok: there's a joke in here somewhere about leaking pipelines

ruaok: agreed

thanks for the discussion on that

ruaok: what's your workflow for after a PR is approved? author merges it? reviewer merges it? you merge everything?

I have no strong feeling at this time, since there are no release cycles yet. so, I just press merge when there are no issues.

but happy to adjust if you'd like to.

I'm just unsure if I should merge #23 myself :)

[troi-recommendation-playground] alastair merged pull request #23 (main…requests-params): Use request's params argument instead of encoding manually https://github.com/metabrainz/troi-recommendati...

I didn't merge because the tests hasn't passed yet.

but once approved test tests passed, feel free to merge.

and we host our own jenkins, yes? we should switch to that -- travis is taking eons to run tests...

yes, I suggested a few days ago that we put it on jenkins. I'll do that now

quick question: what's id here in Area? https://github.com/metabrainz/troi-recommendati...

mbid? row id? the default to that parameter is currently the python builtin `id`, which is almost certainly not correct

area_id as in id in the musicbrainz.area table. I suppose area_id would be a better name for the argument.

should both parameters be required, rather than optional?

I'm making some changes here now, I can update it