distribution is what we have now, so that might be right.
let me upload this and see if it works.
pristine___
The pink tee looks great!
outsidecontext
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.
ruaok
outsidecontext: You seem to have created a new dist cert with expiration 2021/10/28.
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
ruaok
Ok, will look in a minute.
I found out earlier that recording year lookup is quite buggy.
CatQuest has left the channel
CatQuest joined the channel
CatQuest has quit
CatQuest joined the channel
BrainzGit
[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...
ruaok
meh. can't think clearly today.
alastairp
there's this imposing sense of dread sitting over everything this week
ruaok
literally everything.
but me sleeping poorly is my main issue -- I've gotten used to the feeling of dread. #2020
alastairp
I hope that improves for you
ruaok
thx. should be ok. I think that was a fluke.
alastairp
hmm
ruaok
now I am really confused about super and when the base class __init__ need to be called.
alastairp
yeah, I was just going to ask about that
ruaok
it needs to be called for all Elements, but no args need to be passed.
alastairp
correct
ruaok
but unless I explicitly call it, the object does not correctly get initialized.
so, I really need all those calls to super.
alastairp
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__()
ruaok
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.
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.
alastairp
right, so I guess it's making it explicit that this class takes no arguments in its constructor
ruaok
Ok, #21 updated.
#22 looks interesting. how do you test when you need to pass an array of Recordings ?
alastairp
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
ruaok
I see this getting very cumbersome.
alastairp
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
ruaok
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?
alastairp
I don't think I want to construct pipelines from the commandline, it sounds like the parsing for that will get complex really quickly
ruaok
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.
alastairp
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
ruaok
your motivation is that testing an element with real data is too much data flowing for easy debugging. is that right?
sumedh joined the channel
alastairp
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
ruaok
so, what if we hone in on this one specific feature then?
something like a "limited data debugging" mode?
alastairp
ohh, interesting
ruaok
construct pipeline, run pipeline, but terminate once you get 1 data element passed into the target element?
alastairp
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?
ruaok
I suppose.
alastairp
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
reosarevok
So would that be a halfpipe?
🛹
reosarevok shuts up
ruaok
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.
alastairp
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?
ruaok
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.
alastairp
I'm just unsure if I should merge #23 myself :)
BrainzGit
[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...
ruaok
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...
alastairp
oh right, didn't catch that point sorry
yes, I suggested a few days ago that we put it on jenkins. I'll do that now