Skip to content

fix: to_archive does not store resources associated with ingredients #1151

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

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

Conversation

alextrnnn
Copy link
Contributor

@alextrnnn alextrnnn commented Jun 9, 2025

Changes in this pull request

Scenario (detected by unit test):

  • Create a builder from a manifest definition
  • Add an ingredient with a stream to that builder
  • Create an archive from that builder
  • Create a builder from that archive
  • Attempt to sign with the builder

This scenario would result in a ResourceNotFound error.

Modifications to_archive/from_archive to add/read the ingredient's resource files to/from the zip have been added to resolve this error.

Checklist

  • This PR represents a single feature, fix, or change.
  • All applicable changes have been documented.
  • Any TO DO items (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.

@alextrnnn alextrnnn force-pushed the alextrnnn/fix-archive branch from 1355292 to 7782d90 Compare June 9, 2025 20:01
@tmathern
Copy link
Contributor

@alextrnnn Seems there is a conflict to resolve.

Copy link
Contributor

@tmathern tmathern left a comment

Choose a reason for hiding this comment

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

LGTM, but please wait for a review from @gpeacock or @mauricefisher64 regarding the validity of us supporting this scenario. Thanks.

Copy link
Contributor

@ok-nick ok-nick left a comment

Choose a reason for hiding this comment

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

Nice work.

@alextrnnn alextrnnn requested a review from ok-nick June 11, 2025 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants