reosarevok: this line in handleConfirm is suspect - confirmedProps[state.creditableEntityCredit] = state.credit.value;
2021-11-16 32051, 2021
bitmap
state.creditableEntityCredit is undefined there
2021-11-16 32033, 2021
reosarevok
Oh, yeah, I guess I need to pass that to the state on createInitialState
2021-11-16 32036, 2021
monkey
Now that we're adding cover art everywhere, it would be interesting to look at the Save-Data HTTP header, and set a global prop accordingly. Then in React we can decide to show artwork or not.
There's a proposed CSS solution for this as well (which is how I know of it) but in our case a property in React is more adapted. Plus it's not currently supported anywhere…
2021-11-16 32034, 2021
bitmap
reosarevok: I guess props.relationship.creditableEntityCredit works too
2021-11-16 32058, 2021
reosarevok
bitmap: ok, the whole thing does the trick, but now I need to figure out how to actually save the credit :D
2021-11-16 32016, 2021
lucifer
spotify reader emitting lots of 404s. trying to reach spotify.com manually also gives 404. probably a spotify downtime.
Weird, I'm listening to Spotify rn, maybe it just pre-downloaded the podcast :D
2021-11-16 32036, 2021
akshaaatt
I received an email from LB saying that there has been an issue with the import
2021-11-16 32003, 2021
akshaaatt
Does this mean all our users who have authenticated Spotify on LB have received this email?
2021-11-16 32020, 2021
lucifer
i think so. yes.
2021-11-16 32059, 2021
lucifer
that also means that import will eventually be disabled for all users. once spotify is back up, will need to run a script to restart imports for all users
2021-11-16 32011, 2021
monkey didn't receive that email
2021-11-16 32057, 2021
reosarevok
I did
2021-11-16 32008, 2021
reosarevok
Which reminds me, does the Spotify importer also import podcasts?
lucifer: I wonder if we need some better error detection here. perhaps 404 shouldnt trigger thi failure
2021-11-16 32044, 2021
akshaaatt
lucifer is it desirable to continue the entire process once we find a 404 or the script should put a hault? Considering sending emails to a large userbase in a short interval of time could be undesirable I think?
2021-11-16 32011, 2021
monkey
Ah, now I got the email
2021-11-16 32001, 2021
lucifer
alastairp: i think that makes sense. but what to do for those errors, ignore and keep retrying?
2021-11-16 32027, 2021
alastairp
at the very least we could do a back-off
2021-11-16 32051, 2021
alastairp
we had a discussion about this a few months ago, to improve the importer and make it more responsive, especially for now-playings
2021-11-16 32011, 2021
alastairp
and so the same system could help us add a backoff in some cases
2021-11-16 32036, 2021
alastairp
we could back off a few times, and only after it fails many times we quit and send the mail
2021-11-16 32036, 2021
lucifer
akshaaatt: the script halts the import and thus the user is notified. the intent is that the error usually means user intervention is needed. sending a lot of mails is undesirable indeed but not sure about the best way forward here
2021-11-16 32052, 2021
akshaaatt
Right
2021-11-16 32017, 2021
akshaaatt
I'm interested in what alastairp says I think that could be good
2021-11-16 32020, 2021
alastairp
but I think we also now know of a few different failure modes now, and so perhaps should differentiate them
2021-11-16 32021, 2021
lucifer
backing off makes sense.
2021-11-16 32043, 2021
alastairp
the auth failure - we know that this can happen when people revoke permissions
2021-11-16 32006, 2021
alastairp
but others seem to be a bit more transient - 500? 404? ...
2021-11-16 32032, 2021
lucifer
yeah indeed. afair, auth revocation is the only one needing user intervention. all others are transient errors and resolve themselves after some retries.
2021-11-16 32004, 2021
rdswift_ joined the channel
2021-11-16 32018, 2021
alastairp
perfect
2021-11-16 32011, 2021
rdswift has quit
2021-11-16 32019, 2021
rdswift_ is now known as rdswift
2021-11-16 32058, 2021
lucifer
to implement this stuff, i am thinking of a column like is_error_transient flag or error_type enum
2021-11-16 32036, 2021
lucifer
for permanent errors (auth revocation), send email and stop. for transient errors, don't retry for another hour or so? for a few times and then upgrade that to a permanent error and send mail
afaiu importer works for free accounts too, its the playing now feature that needs premium.
2021-11-16 32035, 2021
rdswift
I guess it's been years rather than weeks. :-)
2021-11-16 32009, 2021
rdswift
I guess there's nothing that triggers an update to your database that I'm not connected, although I don't suppose that's an issue.
2021-11-16 32055, 2021
alastairp
rdswift: you could turn it off yourself, but I guess we don't disable it ourselves if we don't see any activity
2021-11-16 32030, 2021
rdswift
Not a big deal either way. I was just a bit surprised when I received the email. Might be an idea to check the last listen date and not send the email if it's over x days?
2021-11-16 32041, 2021
alastairp
yeah, or we could automatically disconnect it after x months, sending an email indicating it
2021-11-16 32058, 2021
navap1 has quit
2021-11-16 32023, 2021
navap1 joined the channel
2021-11-16 32049, 2021
rdswift
Is it a problem leaving it connected?
2021-11-16 32033, 2021
alastairp
a problem for us? not at all
2021-11-16 32059, 2021
rdswift
In that case, I don't think I would bother having you automatically show it as disconnected after x months.
2021-11-16 32018, 2021
rdswift
One less piece of code to run periodically. ;-)
2021-11-16 32025, 2021
alastairp
in this case I'd just do it as part of the infinite loop that we have recording listens anyway... if last_listened_time < 6 months ago: email and disable account, then it won't get caught the next time around
2021-11-16 32038, 2021
alastairp
but you're right, if it's not causing us problems then probably no reason not to
