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

chore: fix typing for manifest_config param of push fn #141

Merged

Conversation

tarilabs
Copy link
Contributor

@tarilabs tarilabs commented Jun 6, 2024

for push() the parameter manifest_config seems to me has the wrong type annotation.

manifest_config: Optional[dict] = None,

while it should be an Optional[str]

considering it's used in push() to be passed to _parse_manifest_ref()

oras-py/oras/provider.py

Lines 785 to 786 in 9527aa1

if manifest_config:
ref, media_type = self._parse_manifest_ref(manifest_config)

which expect a str (and not a dict)

oras-py/oras/provider.py

Lines 228 to 246 in 9527aa1

def _parse_manifest_ref(self, ref: str) -> Tuple[str, str]:
"""
Parse an optional manifest config.
Examples
--------
<path>:<content-type>
path/to/config:application/vnd.oci.image.config.v1+json
/dev/null:application/vnd.oci.image.config.v1+json
:param ref: the manifest reference to parse (examples above)
:type ref: str
:return - A Tuple of the path and the content-type, using the default unknown
config media type if none found in the reference
"""
path_content: PathAndOptionalContent = oras.utils.split_path_and_content(ref)
if not path_content.content:
path_content.content = oras.defaults.unknown_config_media_type
return path_content.path, path_content.content

hope this helps :)

Verified

This commit was signed with the committer’s verified signature.
tarilabs Matteo Mortari
Signed-off-by: tarilabs <matteo.mortari@gmail.com>
@vsoch vsoch merged commit fd8a83c into oras-project:main Jun 6, 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.

None yet

2 participants