Skip to content

Emit fully absolute URIs from jsonschema_suite remotes. #585

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

Merged
merged 3 commits into from
Aug 17, 2022
Merged

Conversation

Julian
Copy link
Member

@Julian Julian commented Aug 10, 2022

I.e. instead of

{
"foo.json": "..."
}

you get

{
"http://localhost:1234/foo.json": "..."
}

which are the URIs the refRemote.json files expect.

This was I believe just an oversight on my part (or I never noticed
it before) -- but there's no reason a user of this should need to
know the base URI is http://localhost:1234/, the whole point of the
command is to just give you the URIs + schemas to configure.

I don't know how many people use this command rather than reading
directly off the remotes/ directory, but I do, so it's at least 1
person.

This was I believe just an oversight on my part (or I never noticed
it before) -- but there's no reason a user of this should need to
know the base URI is http://localhost:1234/, the whole point of the
command is to just give you the URIs + schemas to configure.

I don't know how many people use this command rather than reading
directly off the remotes/ directory, but I do, so it's at least 1
person.
@Julian Julian requested a review from a team as a code owner August 10, 2022 13:21
@karenetheridge
Copy link
Member

Could you add a mention in the README that the base uri is 'http://localhost:1234', so tooling that performs their own processing of 'remotes' can do so correctly?

e.g. this is what I do (which I believe to be correct) - I would use the script but I don't want to impose another prerequisite on python3 on my downstream consumers, as automated test systems especially are likely to not have it.

https://metacpan.org/dist/Test-JSON-Schema-Acceptance/source/lib/Test/JSON/Schema/Acceptance.pm#L132-145

@Julian
Copy link
Member Author

Julian commented Aug 11, 2022

Could you add a mention in the README that the base uri is 'http://localhost:1234/', so tooling that performs their own processing of 'remotes' can do so correctly?

Yep, good idea, done.

@Julian
Copy link
Member Author

Julian commented Aug 17, 2022

I'm assuming this is good to go now with folks but if not obviously follow up.

@Julian Julian merged commit 14d05dc into main Aug 17, 2022
@Julian Julian deleted the absolute-uri branch August 17, 2022 11:08
Julian added a commit to python-jsonschema/jsonschema that referenced this pull request Aug 17, 2022
Julian added a commit that referenced this pull request Aug 17, 2022
This was fixed in my own repo and I just forgot to
bring it over when moving it upstream.
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.

4 participants