-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Issue #2730 - better handle types for expanded variables #2759
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
Changes from all commits
38e62e0
a891447
f52e882
e2a43e2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -386,41 +386,51 @@ def format_error_message(error): | |
|
|
||
|
|
||
| def validate_against_fields_schema(config_file): | ||
| schema_filename = "fields_schema_v{0}.json".format(config_file.version) | ||
| schema = load_jsonschema("fields", config_file.version) | ||
| _validate_against_schema( | ||
| config_file.config, | ||
| schema_filename, | ||
| schema, | ||
| format_checker=["ports", "expose", "bool-value-in-mapping"], | ||
| filename=config_file.filename) | ||
|
|
||
|
|
||
| def validate_against_service_schema(config, service_name, version): | ||
| _validate_against_schema( | ||
| config, | ||
| "service_schema_v{0}.json".format(version), | ||
| load_jsonschema("service", version), | ||
| format_checker=["ports"], | ||
| path_prefix=[service_name]) | ||
|
|
||
|
|
||
| def load_jsonschema(prefix, version): | ||
| schema_filename = "{0}_schema_v{1}.json".format(prefix, version) | ||
| config_source_dir = os.path.dirname(os.path.abspath(__file__)) | ||
|
|
||
| if sys.platform == "win32": | ||
| config_source_dir = config_source_dir.replace('\\', '/') | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure why this is necessary. On windows I would expect
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes, I saw that, but I'm not sure why it's there either. This is mostly just a question, it may be necessary. If you can remove it and the appveyor tests pass (they run on windows) then I think it's safe to remove. |
||
|
|
||
| schema_file = os.path.join(config_source_dir, schema_filename) | ||
|
|
||
| with open(schema_file, "r") as schema_fh: | ||
| schema = json.load(schema_fh) | ||
| return schema | ||
|
|
||
|
|
||
| def _validate_against_schema( | ||
| config, | ||
| schema_filename, | ||
| schema, | ||
| format_checker=(), | ||
| path_prefix=None, | ||
| filename=None): | ||
| config_source_dir = os.path.dirname(os.path.abspath(__file__)) | ||
|
|
||
| config_source_dir = os.path.dirname(os.path.abspath(__file__)) | ||
| if sys.platform == "win32": | ||
| file_pre_fix = "///" | ||
| config_source_dir = config_source_dir.replace('\\', '/') | ||
| else: | ||
| file_pre_fix = "//" | ||
|
|
||
| resolver_full_path = "file:{}{}/".format(file_pre_fix, config_source_dir) | ||
| schema_file = os.path.join(config_source_dir, schema_filename) | ||
|
|
||
| with open(schema_file, "r") as schema_fh: | ||
| schema = json.load(schema_fh) | ||
|
|
||
| resolver = RefResolver(resolver_full_path, schema) | ||
| validation_output = Draft4Validator( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems overly complex. I think supporting arrays is probably the most contentious part of this feature. I think we should try to keep it as straightforward as possible.
How about:
Although, I wonder if it should actually be
yaml.safe_load()since the config is yaml format. Using yaml would support json as well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I said I have not written python for years, but it seems that
json.loadsdoes not work with an Array.Looks like I am not the only one to face the issue
=> http://stackoverflow.com/questions/10973614/convert-json-array-to-python-list
However, it may be just because of issue between simple quotes (') and double quotes (")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's it.
jsononly supports double quotes for strings.