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

Fix delegated delivery: Dont update episodes for mixed feed release #1097

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

svevang
Copy link
Member

@svevang svevang commented Sep 18, 2024

Closes #1080

Copy link
Member

@kookster kookster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@@ -7,6 +7,9 @@ class Api
DEFAULT_BATCH_SIZE = 5
DEFAULT_WRITE_BATCH_SIZE = 1

NOT_FOUND = 404
CONFLICT = 409
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

409, bane of my existence.
great to have this handling for it now!

@@ -171,8 +174,11 @@ def check_row(row_operation)
true
end

def ok_code(resp, ignore_not_found: false)
return true if ignore_not_found && resp.code.to_i == 404
def ok_code(resp, ignore_errors: [])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the reason to pass in the errors to ignore is so we only ignore 409 on some requests - esp updates?
It looks like they are not ignored for when querying/getting data, and are ignored on updates - I wonder if we could even get a 409 for a query? anyway, this make sense, no reason to change it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm, So this may not be quite right.
Because we're proxying requests via the api bridge, this "ok_code" is examining the wrapper's response code, not the DD API responses.

The actual api responses are probed and filtered here https://github.com/PRX/feeder.prx.org/pull/1097/files#diff-3ee82c0e582e70e00c46671634fbf13b219bd4297018576f21fb6f1d4e2133fbR233

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle case where episode has hit public feed prior to delegated delivery upload/upload
2 participants