Remove support for schemas that are loaded from a URL (breaking) - #412
Remove support for schemas that are loaded from a URL (breaking)#412ATorrise wants to merge 4 commits into
Conversation
Signed-off-by: ATorrise <ambertorrise@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #412 +/- ##
==========================================
+ Coverage 83.06% 83.36% +0.29%
==========================================
Files 49 49
Lines 2982 2975 -7
==========================================
+ Hits 2477 2480 +3
+ Misses 505 495 -10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Amber Torrise <112635587+ATorrise@users.noreply.github.com>
zFernand0
left a comment
There was a problem hiding this comment.
LGTM! 😋
I do have one small request about raising the error.
Feel free to get other developer's opinion on this.
| return [] | ||
| # remote schema loading is not supported | ||
| if not self.__suppress_config_file_warnings: | ||
| warnings.warn(f"Loading a JSON schema from a remote URL is not supported: {schema}") |
There was a problem hiding this comment.
We seem to be using this same string in multiple paces... wondering if we could move it somewhere else and reuse it.
| if not self.__suppress_config_file_warnings: | ||
| warnings.warn(f"Loading a JSON schema from a remote URL is not supported: {schema}") | ||
| self.__logger.warning(f"Loading a JSON schema from a remote URL is not supported: {schema}") | ||
| return [] |
There was a problem hiding this comment.
I think we should be raising the error regardless.
Right now, the only place that's calling the schema_list function is inside the get_env function, which is only called when the user specifies that they want to override_with_env.
If we return an empty list here, the developer/user may not realize that their environment variables were not loaded because the schema comes from a URL. This leads to unexpected behavior.
Signed-off-by: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com>
What It Does
Removes support for loading a JSON schema from a remote
http(s)://URL via a config file's$schemaproperty.validate_config_json(used during profile loading) now raises aValueErrorif$schemais anhttp(s)://URL, telling the user to use a local file path instead.ConfigFile.schema_list(used byget_env) now returns an empty list and logs/warns instead of making a network request for a remote$schemaURL.Only local schema files (absolute path, relative path, or
file://) are supported now.How to Test
run the new tests and make sure they pass
Review Checklist
I certify that I have:
Additional Comments
Breaking! Cases where
$schemapoints at a remote url now fail instead of silently succeeding