Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Change behaviour of scrobbler to clear sent scrobbles when successful #105

Merged
merged 3 commits into from
Apr 27, 2018
Merged

Change behaviour of scrobbler to clear sent scrobbles when successful #105

merged 3 commits into from
Apr 27, 2018

Conversation

rikkit
Copy link
Member

@rikkit rikkit commented Aug 23, 2017

WIP - needs to handle potential failure of one batch

@rikkit
Add RemoveFromCache and GetCachedCount methods to IScrobbler and SQLi…
f71140a 
…te implementation

- change scrobbler behaviour to remove sucessfully scrobbled tracks from the cache
- change return value of Scrobble methods - will always be either Successful, Cached. Actual failure reason may be stored by Scrobbler cache impl
- Remove CacheEnabled property from IScrobbler. Just use new MemoryScrobbler class if needed
- Remove Scrobbler class, add MemoryScrobbler. No migration path because the behaviour is different - and the old behaviour not that useful
@SHOEGAZEssb
Copy link
Contributor

any news on this =)

@rikkit
Copy link
Member Author

rikkit commented Sep 19, 2017

Sorry! I got stuck with publishing the package - and I've been off on medical leave for the last couple weeks. So IIRC I just need to figure out how to migrate my current NuGet packaging to the new dotnet CLI, plus handle the error cases. I wonder if you could check out this branch and see if it matches what you wanted?

@SHOEGAZEssb
Copy link
Contributor

Oh, hope youre doing fine! I'll test if it works as expected!

@rikkit
Copy link
Member Author

rikkit commented Sep 19, 2017

Thanks :)

@SHOEGAZEssb
Copy link
Contributor

I had a quick test today, and the cache wasn't cleared after scrobbling.
I debugged a little bit and found that you only remove the tracks that are scrobbled "normally" (not from the cache): See here
Instead of creating the acceptedMap from the scrobblesList, it should be created from the pending object (I think)

@rikkit
Copy link
Member Author

rikkit commented Sep 20, 2017

Ah good catch! Thanks for checking, I'll add that to the unit tests. Hopefully I can get this all wrapped up this weekend 👍

This was referenced Nov 15, 2017
Merged
Open
@rikkit rikkit added this to the 1.0.1 milestone Nov 15, 2017
@rikkit rikkit merged commit 9f11c2e into master Apr 27, 2018
@rikkit rikkit mentioned this pull request Jul 1, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.1
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@rikkit @SHOEGAZEssb