Skip to content

Conversation

mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented Jan 30, 2025

This is #2230 cherry-picked for the release-1.57 branch.

The original PR was LGTMed by @giuseppe , but is not yet merged to main.

Try to split fields by purpose, and document some of the context.

For consistency, use the same order in the struct literals.

Only reorders existing fields, should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Instead, have the callees produce ErrFallbackToOrdinaryLayerDownload
directly.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
That's a logically better place, it pairs the getBlobAt
calls with the ErrBadRequest types specific to those call sites.

We will, also, add more fallback reasons.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... but not if the fallback would be convert_images, again
creating too large metadata.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Copy link
Contributor

openshift-ci bot commented Jan 30, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mtrmac

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac mtrmac changed the title [1.57] Fall back from chunked pulls when the chunked metadata is too large [1.57] Fall back from chunked pulls when the chunked metadata is too large and tag as 1.57.1 Jan 30, 2025
@mtrmac
Copy link
Collaborator Author

mtrmac commented Jan 30, 2025

Updated to also include the version bump to 1.57.1.

@TomSweeneyRedHat
Copy link
Member

LGTM
once #2330 merges without change.

@mtrmac
Copy link
Collaborator Author

mtrmac commented Jan 30, 2025

Alternatively, #2241 also includes the limit bump from #2240. I really don’t care which of the two variants of 1.57.1 is merged.

@TomSweeneyRedHat
Copy link
Member

This one is happy and ready to merge with another LGTM. I'd like to get the update from #2240 in also, but #2241 is having a lint issue of some sort.

@TomSweeneyRedHat
Copy link
Member

I'd prefer to hold for #2241, it's in test mode at the moment.

@TomSweeneyRedHat
Copy link
Member

Closing in favor of #2241

@mtrmac mtrmac closed this Jan 30, 2025
@mtrmac mtrmac deleted the chunked-too-large-1.57 branch January 30, 2025 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants