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

feat: simplify how subjects are created from existing manifest #131

Merged
merged 1 commit into from
May 22, 2024

Conversation

tumido
Copy link
Contributor

@tumido tumido commented May 2, 2024

Hi folks 👋

I've noticed the Python Oras client library has kind of rudimentary support for attached artifacts. I'd like to address a couple of UX caveats in several PRs. Please let me know if it's heading in the right direction or if you have a different perspective.

This PR addresses a difficulty when creating a Subject reference.

As of today, an oras-py user needs to know how the subject spec is constructed, and how to calculate size and digest. This makes it quite hard to come up with a usable structure that can be used when pushing an artifact. I've addressed this by adding a from_manifest class method to the Subject data class. There may be a nicer, cleaner solution than a class method, let me know what you think.

Copy link
Contributor

@vsoch vsoch left a comment

Choose a reason for hiding this comment

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

I like how this works! The reason the support is simple is because the interaction is more complex than seems at face value. If we want to fully support a client doing this interaction, the full thing is described here: https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pushing-manifests-with-subject Let me know your thoughts.

docs/getting_started/user-guide.md Outdated Show resolved Hide resolved
oras/provider.py Outdated Show resolved Hide resolved
@tumido
Copy link
Contributor Author

tumido commented May 2, 2024

If we want to fully support a client doing this interaction, the full thing is described here: opencontainers/distribution-spec@main/spec.md#pushing-manifests-with-subject Let me know your thoughts.

Well... I kinda don't like when things are half-done. 🙂 So.. I'll look into it and try to improve the push logic. (I may not have much time in the next two weeks though...)

Do you think it would make sense to split the push logic into push and attach methods, similarly to what oras CLI does? https://oras.land/docs/commands/oras_attach

@tumido tumido force-pushed the simplify-subject-creation branch 2 times, most recently from 00c2de4 to 2fca120 Compare May 2, 2024 15:17
@vsoch
Copy link
Contributor

vsoch commented May 2, 2024

Do you think it would make sense to split the push logic into push and attach methods, similarly to what oras CLI does? https://oras.land/docs/commands/oras_attach

Since we don't have a command line client, let's start with what you have now - a push that takes a subject. If "attach" comes down to just doing that command again, we don't need attach. But if there is another use case not covered, we can consider adding it.

@tumido tumido requested a review from vsoch May 21, 2024 14:21
@tumido
Copy link
Contributor Author

tumido commented May 21, 2024

@vsoch does anything have to change in this PR or is this fine? 🙂

@vsoch
Copy link
Contributor

vsoch commented May 21, 2024

You'll want to finish up by:

  • Bump the version in oras/version.py
  • Adding a line to the CHANGELOG.md with the change and new version

@tumido tumido force-pushed the simplify-subject-creation branch from 2fca120 to 5db7ebe Compare May 22, 2024 11:22
@tumido
Copy link
Contributor Author

tumido commented May 22, 2024

Done, thank you. 🙂

@vsoch vsoch merged commit e5d2307 into oras-project:main May 22, 2024
5 checks passed
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.

2 participants