-
Notifications
You must be signed in to change notification settings - Fork 15
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
Upload data to dataset using readable object #386
Upload data to dataset using readable object #386
Conversation
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 had a minor question and then a requested change. We should be able to add an implementation for DatasetInternalClient without too much trouble, so let's include it here.
``` | ||
""" | ||
|
||
def read(self, size: Union[int, None] = ...) -> bytes: |
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.
Any particular reason for Union[int, None]
instead of Optional[int]
?
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.
Nothing in particular, I just took inspiration from python's BufferedReader interface:
def read(self, __size: int | None = ...) -> bytes: ...
@@ -256,6 +280,10 @@ async def upload_to_dataset(self, dataset_name: str, paths: List[Path]) -> bool: | |||
# TODO: ********************************************* | |||
raise NotImplementedError('Function upload_to_dataset not implemented') | |||
|
|||
async def upload_data_to_dataset(self, dataset_name: str, item_name: str, data: AsyncReader) -> bool: | |||
# TODO | |||
raise NotImplementedError('Function upload_data_to_dataset not implemented') |
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 correct internal equivalent to MaaSDatasetManagementMessage is just DatasetManagementMessage. MaaSDatasetManagementMessage is separate only (as far as I remember) to give us a separate subtype that also inherits from ExternalRequest, to get things related to authentication, etc. I don't exactly know why MaaSDatasetManagementMessage has those two additional attributes.
We really should include an implementation for this at the same time. You should be able to copy the other for the most part, making a few adjustments and removals for things related to external auth.
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.
Thanks for the guidance. Ill make that addition and let you know if I run into any roadblocks.
Closing this in favor of #540. |
Adds an async
upload_data_to_dataset
method to dataset clients that polls an async readable object that implementsasync def read(self, size: Optional[int] = None) -> bytes
. This adds flexibility, so it is not required to treat to-be uploaded items as files.Additions
upload_data_to_dataset
method to dataset clients that polls an async readable object that implementsasync def read(self, size: Optional[int] = None) -> bytes
.Notes
MaaSDatasetManagementMessage
Todos
upload_data_to_dataset
forDatasetInternalClient