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.

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

Scan acts as it just inserted brand new records #57

Open
w0rd-driven opened this issue Mar 15, 2023 · 1 comment
Open

Scan acts as it just inserted brand new records #57

w0rd-driven opened this issue Mar 15, 2023 · 1 comment
Assignees
Labels
bug Something isn't working
Milestone

Comments

@w0rd-driven
Copy link
Owner

Scanning is no longer idempotent or it possibly never was. After running a verify and a subsequent scan, I recently noticed how the images and verified_at timestamps were removed. I had wondered why artist scans for id 4 would seemingly verify the entire collection again. It is problematic to use the worker that is very much specialized for a single backfill operation. It's not a bad thing to retry the failed lookups but that list will eventually grow indefinitely.

I was able to reproduce this precisely and I have a feeling it comes back to the upsert operation. It's perfect for a single shot bulk insert but the timestamp placeholders create what look like new records instead of look for a whitelist. We only want it to update certain fields, not essentially roll back changes. This is a neat trick and would essentially count as starting over.

@w0rd-driven w0rd-driven added the bug Something isn't working label Mar 15, 2023
@w0rd-driven w0rd-driven self-assigned this Mar 15, 2023
@w0rd-driven w0rd-driven added this to the Public beta milestone Mar 15, 2023
@w0rd-driven
Copy link
Owner Author

This is a big enough known issue and blocker tbh but I do want to release the repo anyway. What absolutely should be covered in this is to work through the Oban worker and the verify function. Why do I return a notification again? The broadcast sends it as an event and the record shouldn't do anything special.

It'll be good to have a full on refactor of this whole flow from top to bottom. The current UI having a verify() also save the artist record like the Oban job is awkward. I want to stick to SRP but I think prior languages are influencing how I break things up. Updating the timestamp is absolutely part of this call and I need to make sure data flows through the pipe in predictable ways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant