fixed typing that API uses #1332
Conversation
andrewelamb
commented
Nov 28, 2023
andrewelamb
commented
- Fixes fds-1342
- Fixes incorrect type hinting (parameters and return types) for methods that the API uses.
| @@ -292,13 +298,13 @@ def submit_metadata_manifest( | |||
| dataset_id: str, | |||
| manifest_record_type: str, | |||
| restrict_rules: bool, | |||
| validate_component: str = None, | |||
| access_token: str, | |||
There was a problem hiding this comment.
should this one be Optional[str] = None because it used to be str = None?
There was a problem hiding this comment.
For access_token I removed the default of None because I was under the impression that you always needs an access token to submit since you are storing a manifest.
There was a problem hiding this comment.
Ahh right, I think I had thought this was the validation method. thanks for clarifying
There was a problem hiding this comment.
@andrewelamb @GiaJordan We need to keep the None default, since this change actually broke the CLI submit if you require the access token. The CLI gets the access token another way from the API. I will submit another PR to update typing Optional[str]=None
schematic/models/metadata.py
Outdated
| @@ -292,13 +298,13 @@ def submit_metadata_manifest( | |||
| dataset_id: str, | |||
| manifest_record_type: str, | |||
| restrict_rules: bool, | |||
| validate_component: str = None, | |||
| access_token: str, | |||
| validate_component: str = Optional[None], | |||
There was a problem hiding this comment.
is this backwards or intentional?
There was a problem hiding this comment.
Oopps, yep this is a mistake I'll fix this.
