yeah, so I think that the epoch timestamp was a mistake
2021-04-12 10238, 2021
alastairp
where at some point in time we did `ujson.dumps(somelisten.tojson())`
2021-04-12 10240, 2021
ruaok
to_json returns a dict, yes.
2021-04-12 10249, 2021
alastairp
and it turned the datetime object into an epoch timestamp
2021-04-12 10211, 2021
ruaok
at some point we need to clean up the listen object and get rid of one or the other.
2021-04-12 10212, 2021
alastairp
and then we looked at it and said "oh, we need an iso-formatted date too", and added that in as well
2021-04-12 10250, 2021
alastairp
when it was actually already there, but had just been implicitly converted without us knowing
2021-04-12 10255, 2021
alastairp
anyway, just a suspicion
2021-04-12 10213, 2021
alastairp
but I agree, some good low hanging fruit to clean up there, and as you said some optimisation in the ingestion pipeline too
2021-04-12 10215, 2021
alastairp
did I tell you about the optimisation I did for someone's shopify app? where the shopify library spent more time decoding json to python objects than it did making the http request and decoding the json D:
2021-04-12 10216, 2021
ruaok
and it was a one character fix and a one line addition in requirements.txt ?
2021-04-12 10237, 2021
alastairp
I ended up removing the library and just using requests instead. it only needed a few pieces of data from the entire response
2021-04-12 10246, 2021
alastairp
yeah, suddenly everything is 2x faster
2021-04-12 10235, 2021
ruaok
oh, I did some cleanup on bono data sets after deploying the latest changes to labs.api -- there are no more duplicates between the two now
I think its time to define a coding standard for our python projects and at the same time define configurations for common tools such as autopep8 so that every developer doesn't have to reinvent the wheel. and I really appreciate it it that you're so gung ho to write this up. much appreciated!
2021-04-12 10232, 2021
ruaok
!m alastairp
2021-04-12 10232, 2021
BrainzBot
You're doing good work, alastairp!
2021-04-12 10213, 2021
alastairp
sure, _lucifer had some ideas for that already too, we'll look at combining everything together
2021-04-12 10234, 2021
ShivamAwasthi joined the channel
2021-04-12 10235, 2021
alastairp
I do appreciate that you're taking our feedback on board about this
2021-04-12 10215, 2021
ruaok
I've never had any objections. it just was never clear what our common goal was, which makes it hard to aim for that goal.
2021-04-12 10215, 2021
ShivamAwasthi
Mr_Monkey, I have made some changes to my proposal. Please review it if you get some free time. =D
2021-04-12 10230, 2021
Mr_Monkey
Will do
2021-04-12 10237, 2021
alastairp
yeah, we mentioned a few times that we have configs and ideas in a few different places. let's get it right once and apply it over everything
2021-04-12 10249, 2021
ruaok
grand!
2021-04-12 10201, 2021
alastairp
ruaok: do you have a local config file that you're using? or is this just the defaults?
alastairp: feedback on 1347 addressed and fixes pushed.
2021-04-12 10207, 2021
sumedh joined the channel
2021-04-12 10232, 2021
_lucifer
alastairp: regarding https://github.com/metabrainz/listenbrainz-server…, ideally we would want to delete only spotify tokens for this. that complicates reseting the sequence. but during testing i guess only some of us will link youtube accounts to test so maybe its fine to delete all tokens to make reseting the sequence simpler. thoughts?
2021-04-12 10256, 2021
yyoung
Hi yvanzo, do you have any other feedback on my proposal? The deadline is around the corner so I'm planning to wrap it up :)
2021-04-12 10236, 2021
alastairp
_lucifer: it's not important to reset the sequence. let's leave that file as-is and we can make another migrate script to delete just spotify accounts before running this script again to re-copy them
2021-04-12 10202, 2021
alastairp
ruaok: cool. do you have a test location to generate a test dump, or do you think it's ready to deploy?
yyoung: for example documentation is still listed as stretch goal, but more important what do you mean with "transplant"?
2021-04-12 10258, 2021
bitmap
then limit which ones can be created for now in code
2021-04-12 10211, 2021
yvanzo
bitmap: ok, I took that ticket.
2021-04-12 10223, 2021
bitmap
thanks!
2021-04-12 10220, 2021
yyoung
yvanzo: Oh I'm sorry I didn't update the stretch goals, but what I mean here is to add documentation to existing code, and I stated "all coding tasks will include tests and documentation" at the beginning of the schedule
2021-04-12 10253, 2021
yyoung
By 'transplant' I mean adapting the current code and logic to the new interface
2021-04-12 10219, 2021
alastairp
_lucifer: 2 minutes to build all images and run tests!
2021-04-12 10226, 2021
_lucifer
yup :D
2021-04-12 10228, 2021
yvanzo
bitmap: yyoung needed more info about React conversion of relationship editing dialog, so as to use it for URL relationship editing. Did you plan to make it available by GSoC coding period? Or does it require more work and may yyoung help you with it?
2021-04-12 10241, 2021
_lucifer
do you think we should bother adding a cache or let it simple?
2021-04-12 10250, 2021
alastairp
I think we should cache the build
2021-04-12 10259, 2021
alastairp
but we probably don't need to cache the sample db image
2021-04-12 10218, 2021
alastairp
(caching build on LB will be super helpful, so it'll be good to test it here)
2021-04-12 10254, 2021
bitmap
yvanzo: yes, it will almost certainly be ready by that time. for URL relationship editing I believe it's already usable
2021-04-12 10257, 2021
_lucifer
yeah the sample db image is just downloaded. so doesn't make sense to cache.
2021-04-12 10218, 2021
bitmap
the only unfinished bits are in the release relationship editor
2021-04-12 10229, 2021
alastairp
well, it depends on how fast download is. if it took 2 minutes to download and 10 seconds to load from cache then cache would be good
2021-04-12 10237, 2021
alastairp
but it seems like it takes 10 seconds to download :)
2021-04-12 10238, 2021
yvanzo
yyoung: week 1: is 'ExternalLinkList' a replacement for the current 'ExternalLinksEditor'?
2021-04-12 10253, 2021
alastairp
check my comment about pull limits though, that's definitely an important consideration
2021-04-12 10211, 2021
_lucifer
yes, i'll add the docker login
2021-04-12 10227, 2021
yyoung
Yes, since in the new interface external links are shown in list (or table?)
2021-04-12 10233, 2021
alastairp
> Secrets are not passed to workflows that are triggered by a pull request from a fork. Learn more.
2021-04-12 10254, 2021
yvanzo
yyoung: the current editor is show in table already, maybe keep the same name?
2021-04-12 10218, 2021
yvanzo
show+n
2021-04-12 10222, 2021
alastairp
I didn't know about this. It means that the action should skip login if the secret isn't available. there's a small risk that in this case the action will fail due to pull limits, but I think that's acceptable
2021-04-12 10208, 2021
yyoung
yvanzo: Did you mean the URL input box?
2021-04-12 10243, 2021
yvanzo
bitmap: thanks, so at worst, we will update URL editing fields for all entity types but release, right?
2021-04-12 10244, 2021
_lucifer
ah yes, that's to avoid leaking secrets from malicious users.
2021-04-12 10223, 2021
alastairp
yeah
2021-04-12 10201, 2021
bitmap
yvanzo: I'm a bit unclear on what the plan is. is it to allow opening the relationship editing dialog in the external links editor as a kind of advanced interface?
2021-04-12 10222, 2021
bitmap
in that case it should work for any entity type, even releases
2021-04-12 10243, 2021
yyoung
yvanzo: OK I got your idea, let's keep the ExternalLinksEditor :)
2021-04-12 10243, 2021
yvanzo
bitmap: yes
2021-04-12 10214, 2021
bitmap
ok, there should be no problem then
2021-04-12 10203, 2021
alastairp
_lucifer: hmm, I just noticed that for the 4gb sample db image, the db layer takes only 1gb. There's some more space here to optimise the rest of the image if we want to reduce the space, possibly we could remove another 1-2gb (this would be from the metabrainz/musicbrainz-test-database image)
2021-04-12 10210, 2021
yvanzo
yyoung: "transplant" implies moving code or logic, which is not the case here IIUC.
2021-04-12 10254, 2021
yvanzo
URLCleanup should be improved to support multiple potential relationship types per entity type.
2021-04-12 10259, 2021
_lucifer
alastairp: yeah, i saw that. that was one of the reasons i asked the difference between the two.
