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

Support reading schemas and instances from pipes rather than files #176

Closed
Julian opened this issue Nov 1, 2022 · 6 comments · Fixed by #180
Closed

Support reading schemas and instances from pipes rather than files #176

Julian opened this issue Nov 1, 2022 · 6 comments · Fixed by #180
Labels
bug Something isn't working enhancement New feature or request

Comments

@Julian
Copy link
Member

Julian commented Nov 1, 2022

In the old CLI, this works:

jsonschema -i <(printf '12') <(printf '{"type": "string"}')

where we're sending JSON in without using a temporary file using zsh's process substitution syntax.

But check-jsonschema doesn't seem to like that:

  check-jsonschema --default-filetype json --schemafile <(printf '{"type": "integer"}') <(printf '12')                           julian@Airm ●
Several files failed to parse.
  Failed to parse /dev/fd/12

and requires using "real files" (by changing the above from <() to =() syntax).

It also seems like it's worth a thought (if you haven't already) at making --default-filetype set to JSON by default so that one can skip passing it in this case, but maybe that's arguable.

@sirosen sirosen added enhancement New feature or request bug Something isn't working labels Nov 1, 2022
@sirosen
Copy link
Member

sirosen commented Nov 1, 2022

Huh! I wouldn't have expected this to fail. At first I labeled this an enhancement but on second thought it feels to me like a bug.
I'll have to play around with this and see why it's not working when I have time to work on this.

It also seems like it's worth a thought (if you haven't already) at making --default-filetype set to JSON by default so that one can skip passing it in this case, but maybe that's arguable.

I don't recall giving this any serious thought. I'll get to that for sure, possibly even before addressing the pipe handling issue. It seems like an easy and innocuous change while this project is still at 0.x .

@sirosen
Copy link
Member

sirosen commented Nov 7, 2022

I finally got some time yesterday afternoon to look at this and I am in that funny situation of "I fixed it, but my tests don't work".
Just to share, I made my work branch visible here: https://github.com/python-jsonschema/check-jsonschema/compare/handle-pipes
There appear to be two issues, one with peeking at the file (identify) and one with pathlib.Path.resolve() on these files.

I'm curious, do you know how those <() syntax files work? I'm getting different -- but also bad! 😅 -- behavior when I use fifos to test this.

@Julian
Copy link
Member Author

Julian commented Nov 7, 2022

Thanks for having a look and I sympathize :D

I may get a few minutes to look at the branch later -- to answer:

I'm curious, do you know how those <() syntax files work? I'm getting different -- but also bad! 😅 -- behavior when I use fifos to test this.

man zshall says:

In the case of the < or > forms, the shell runs the commands in list as a subprocess of the job executing the shell command line. If the system supports the /dev/fd mechanism, the command argument is the name of the device file corresponding to a file descriptor; otherwise, if the system supports named pipes (FIFOs), the command argument will be a named pipe.

so seems like it's OS dependent maybe...

@sirosen
Copy link
Member

sirosen commented Nov 11, 2022

I filed python/cpython#99390 because I think this turns out to be a pathlib bug.
I'd love to suggest a cpython fix, but for now I'm going to special-case files in /proc/<self|pid>/fd/ to avoid this behavior.

Now that I've fixed up the filetype detection to not read files unnecessarily, that seems to be sufficient to resolve this once I release it.

sirosen added a commit that referenced this issue Nov 11, 2022
Files created with this mechanism should not be resolved by
pathlib.Path.resolve. The results are not a valid path to the file.

resolves #176
sirosen added a commit that referenced this issue Nov 11, 2022
Files created with this mechanism should not be resolved by
pathlib.Path.resolve. The results are not a valid path to the file.

resolves #176
sirosen added a commit that referenced this issue Nov 11, 2022
Files created with this mechanism should not be resolved by
pathlib.Path.resolve. The results are not a valid path to the file.

resolves #176
@sirosen
Copy link
Member

sirosen commented Nov 11, 2022

I just published v0.19.1 with the fix, and it works when I test it on your original usage (without --default-filetype needed! 🥳 ).

$ check-jsonschema --schemafile <(printf '{"type": "integer"}') <(printf '12')
ok -- validation done
$ check-jsonschema --schemafile <(printf '{"type": "integer"}') <(printf '"hi"')
Schema validation errors were encountered.
  /proc/self/fd/17::$: 'hi' is not of type 'integer'

Thanks for letting me know about these issues. Let me know again if anything doesn't work for you.

@Julian
Copy link
Member Author

Julian commented Nov 11, 2022

Hooray! Thank you, looks great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants