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 sync failure on unreachable namespace avatar #1544

Merged
merged 1 commit into from
Aug 9, 2023

Conversation

gerrod3
Copy link
Contributor

@gerrod3 gerrod3 commented Aug 2, 2023

fixes: #1543

@gerrod3 gerrod3 force-pushed the logo-404 branch 2 times, most recently from 6f01a8d to 1067efb Compare August 2, 2023 20:16
except ClientError as e:
# Reset DA so that future stages can properly handle it
self.artifact = artifact_copy
self.deferred_download = True
Copy link
Member

Choose a reason for hiding this comment

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

It reminds me a lot of https://github.com/pulp/pulp_deb/blob/main/pulp_deb/app/tasks/synchronizing.py#L192 with https://github.com/pulp/pulp_deb/blob/main/pulp_deb/app/tasks/synchronizing.py#L513 .

Can you elaborate more on the strategy? Do we handle the namespace as if no avatar existed, or do we turn the artifact into on-demand?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is very similar. The current strategy I am using is to handle the namespace as if no avatar existed. In the custom content saver stage I modify the namespace's metadata before save if the avatar failed to download.

Copy link
Member

Choose a reason for hiding this comment

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

Would it help to elevate the FailsafeDeclarativeArtifact to pulpcore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably, we could design it to accept a list of exceptions during download that would be failsafe.

pulp_ansible/app/tasks/collections.py Show resolved Hide resolved
pulp_ansible/app/tasks/collections.py Outdated Show resolved Hide resolved
@mdellweg mdellweg merged commit f56e097 into pulp:main Aug 9, 2023
@gerrod3 gerrod3 deleted the logo-404 branch August 9, 2023 14:59
@patchback
Copy link

patchback bot commented Aug 9, 2023

Backport to 0.17: 💚 backport PR created

✅ Backport PR branch: patchback/backports/0.17/f56e0974df20a53f778cf217fd4e7f64adca19c7/pr-1544

Backported as #1551

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Aug 9, 2023
gerrod3 added a commit that referenced this pull request Aug 9, 2023
fixes: #1543
(cherry picked from commit f56e097)

Co-authored-by: Gerrod <gerrodubben@gmail.com>
@mdellweg
Copy link
Member

AAP-14090

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.

Sync should not fail if namespace's avatar fails to download
2 participants