-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat: Add Client for File Service #65
Conversation
…nd keep the decorated method simple - enum, bool.
@adamarnesen refactored and added all key APIs for FileClient |
… API calls to work around rate limiter
return IteratorFileLike(response.iter_content(chunk_size=4096)) | ||
|
||
|
||
@retry(when=retry.when.status(429), stop=retry.stop.after_attempt(5)) |
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.
@spanglerco Do you know if we have guidance/conventions for HTTP resilience strategies? How many times should we retry?
class FileQueryResponse(JsonModel): | ||
"""The result of a file query""" | ||
|
||
field_links: Dict[str, Link] = Field(alias="_links") |
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.
Why can't we just call this field links
? Also, I wonder if we should even return these links to users. The only way you could use them is by making a manual HTTP request, which kind of defeats the purpose of having the client.
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.
The bottleneck to leave out _links
is that the conftest.py
forbids extra fields in the response compared to the Pydantic model.
nisystemlink-clients-python/conftest.py
Line 91 in 8bc8f63
JsonModel.Config.extra = Extra.forbid |
Example - {"Name": "myfile.txt", "MyProperty": "Value"} | ||
""" | ||
|
||
meta_data_revision: Optional[int] = Field(None, alias="metaDataRevision", example=1) |
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.
metadata
isn't typically broken into two words
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.
It looks like the API implemented it as metaDataRevision
and hence only meta_data_revisio
n will result in a working automatic case conversion.
return parts[-1] | ||
|
||
|
||
def rename_file(client: FileClient, file_id: str, name: str) -> None: |
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.
Our clients have been mostly 1 to 1 with the HTTP API, but I think I'd rather this be a method on the FileClient itself instead of a separate function.
… from URI of `upload_file`
What does this Pull Request accomplish?
Why should this Pull Request be merged?
Improve Developer Experience by using
nisystemlink-clients
instead ofsystemlink-sdk
, which is installed with the SystemLink Client application.What testing has been done?