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 bug where a dict with an id causes contained refs to fail to resolve #1716

Merged
merged 1 commit into from
May 8, 2024

Conversation

braingram
Copy link
Contributor

@braingram braingram commented Dec 21, 2023

Description

Remove use of json_id in find_references callback passed to walk_and_modify.

The json_id argument (which stores the most recently encountered 'id' value) is primarily (perhaps only) useful for schema parsing. However, find_references (called on the ASDF tree) uses the json_id value to construct the uri for the referred object. I see no reference to this behavior in:
https://datatracker.ietf.org/doc/html/rfc6901
https://datatracker.ietf.org/doc/html/draft-pbryan-zyp-json-ref-03
https://datatracker.ietf.org/doc/html/rfc3986
and this can lead to reference resolution failure (see #1715).

Fixes #1715

Checklist:

  • pre-commit checks ran successfully
  • tests ran successfully
  • for a public change, a changelog entry was added
  • for a public change, documentation was updated
  • for any new features, unit tests were added

Copy link
Member

@zacharyburnett zacharyburnett left a comment

Choose a reason for hiding this comment

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

LGTM if tests pass

@braingram
Copy link
Contributor Author

The 2 downstream failures are unrelated to this PR and are addressed in: #1785

@braingram braingram merged commit ffee4b3 into asdf-format:main May 8, 2024
44 of 46 checks passed
@braingram braingram deleted the fix_ref_in_id branch May 8, 2024 16:51
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.

Tree containing id causes reference resolution failure.
2 participants