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

Implement acceptance of custom file extensions for files to validate #119

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

sdruskat
Copy link

This pull request adds the following cli options:

  • -y EXT, --yaml-extension EXT
  • -j EXT, --json-extension EXT

Both accept a string representing a file extension for files like, e.g., my-data-file.ext which contain JSON or YAML data respectively, but don't share the default file endings.

Usage examples:

pykwalify -d my-json-data-file.jext -s schema.yml -j jext
pykwalify -d my-yaml-data-file.yext -s schema.yml -y yext

@coveralls
Copy link

coveralls commented Dec 13, 2017

Coverage Status

Coverage increased (+0.04%) to 87.241% when pulling d0fa7a3 on sdruskat:pykwalify into e7fac6a on Grokzen:master.

@coveralls
Copy link

coveralls commented Dec 13, 2017

Coverage Status

Coverage increased (+0.04%) to 87.241% when pulling 54d967b on sdruskat:pykwalify into e7fac6a on Grokzen:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 87.241% when pulling 54d967b on sdruskat:pykwalify into e7fac6a on Grokzen:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 87.241% when pulling 54d967b on sdruskat:pykwalify into e7fac6a on Grokzen:master.

@coveralls
Copy link

coveralls commented Dec 13, 2017

Coverage Status

Coverage increased (+0.04%) to 87.241% when pulling 6622b08 on sdruskat:pykwalify into e7fac6a on Grokzen:master.

@Grokzen
Copy link
Owner

Grokzen commented Mar 13, 2018

@sdruskat Just a random thought. Why not just remove the blocks that is in place right now to only support .yaml/.json filetypes? Why not just accet any file that you can point to and try to read it either as json or yaml? This feels like a wrong turn somewhere to add another flag to allow more file endings, the cli syntax feels clunky and not correct in some way.

I see no reason right now to merge this MR. Simply allowing any file extension and handle the loading error seems easier.

@dhimmel
Copy link
Contributor

dhimmel commented May 9, 2018

Simply allowing any file extension and handle the loading error seems easier.

Isn't JSON a subtype of YAML? Hence perhaps you'll always be able to read every file using using the YAML parser? Is it ever necessary to fallback to the JSON parser assuming a YAML parser is installed?

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