-
Notifications
You must be signed in to change notification settings - Fork 24
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
Support upload to S3 bucket #287
Conversation
a51ab3f
to
fae3b30
Compare
src/resdk/uploader.py
Outdated
"""The uploader class. | ||
|
||
It is used to upload the file to the Genialis platform. Is supports | ||
uploading files directly to the server and also directly to the S3 bucket. |
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.
Remove "directly" in front of "to the server".
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.
OK.
src/resdk/uploader.py
Outdated
try: | ||
self._upload_config = self.resolwe.api.upload_config.get() | ||
except ResolweServerError: | ||
self._upload_config = {"type": "LOCAL"} |
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.
Add an error log.
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.
OK.
try: | ||
return UploadType[self.upload_config["type"]] | ||
except KeyError: | ||
return UploadType.default() |
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.
Error log.
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.
Done.
2458630
to
8f22270
Compare
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.
For me it works on both: QA and QA2
src/resdk/uploader.py
Outdated
""" | ||
|
||
def __init__(self, resolwe: "Resolwe"): | ||
"""Store the .""" |
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.
Unfinished docstring.
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.
Fixed.
src/resdk/uploader.py
Outdated
UploadType.S3: self._upload_s3, | ||
} | ||
# Use the following upload type if one can not be determined. | ||
self._default_upload_type = UploadType.LOCAL |
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.
Maybe rather UploadType.default()
?
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 have removed this line. It is not needed anymore since the default has been moved to the enum.
src/resdk/uploader.py
Outdated
self._upload_config = self.resolwe.api.upload_config.get() | ||
except ResolweServerError: | ||
self.resolwe.logger.exception("Upload config could not be retrieved.") | ||
self._upload_config = {"type": "LOCAL"} |
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.
Maybe rather {"type": UploadType.default()}
?
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.
Almost: UploadType.default().name
to return the compatible config.
tests/unit/test_resolwe.py
Outdated
@@ -387,7 +393,8 @@ def test_always_ok(self, resolwe_mock): | |||
status_code=200, **{"json.return_value": requests_response} | |||
) | |||
|
|||
response = Resolwe._upload_file(resolwe_mock, self.file_path) | |||
response = Uploader(resolwe_mock)._upload_local(self.file_path) | |||
# response = Resolwe._upload_file(resolwe_mock, self.file_path) |
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.
Remove if not needed.
@@ -398,14 +405,16 @@ def test_always_bad(self, resolwe_mock, requests_mock): | |||
# Immitate response form server - always status 400 | |||
requests_mock.post.return_value = MagicMock(status_code=400) | |||
|
|||
response = Resolwe._upload_file(resolwe_mock, self.file_path) | |||
response = Uploader(resolwe_mock)._upload_local(self.file_path) | |||
# response = Resolwe._upload_file(resolwe_mock, self.file_path) |
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.
Remove if not needed.
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.
Done.
8f22270
to
80622c0
Compare
EXP-370