✨♻️ (⚠️ devops) Remove 5gb limit when uploading data via nodeports #2993
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2993 +/- ##
=======================================
Coverage 79.7% 79.7%
=======================================
Files 693 698 +5
Lines 29049 29215 +166
Branches 3744 3755 +11
=======================================
+ Hits 23156 23294 +138
- Misses 5058 5082 +24
- Partials 835 839 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
GitHK
changed the title
| @@ -44,7 +44,7 @@ def _ensure_remove_bucket(client: Minio, bucket_name: str): | |||
| @pytest.fixture(scope="module") | |||
| def minio_config( | |||
| docker_stack: Dict, testing_environ_vars: Dict, monkeypatch_module: MonkeyPatch | |||
| ) -> Dict[str, str]: | |||
| ) -> Dict[str, Any]: | |||
There was a problem hiding this comment.
very minor: python 3.9 allows to use dict instead of Dict
There was a problem hiding this comment.
Very nice to know!
@colinRawlings you were wondering about None in typing. This is similar.
| formatted_error = "".join( | ||
| [f"\n{_SEP}{k}{_SEP}\n{v}" for k, v in fields.items()] | ||
| ) | ||
| logger.debug("Error serialized to client:%s", formatted_error) |
There was a problem hiding this comment.
question: as far as I see, this creates a text error. Could we not have a JSON formatted one like everywhere else?
There was a problem hiding this comment.
I have it formatted in an easy readable way. This is intended to be used for development/debugging. I'm not sure that having it formatted as json will help us.
There was a problem hiding this comment.
it is just a matter of consistency actually. it will also prevent us from having to manage both text/json cases when parsing the responses
| class RCloneSettings(BaseCustomSettings): | ||
| R_CLONE_S3: S3Settings = Field(auto_default_from_env=True) | ||
| R_CLONE_PROVIDER: S3Provider | ||
| R_CLONE_REGION: str = Field("us-east-1", description="S3 region to use") |
There was a problem hiding this comment.
while I agree it's currently only used by RClone, it is still a S3 setting. and it will be used in AWS deployments cause currently we probably use some default. Therefore I would put it there. Can you please check how storage connects to AWS S3 then?
packages/simcore-sdk/src/simcore_sdk/node_ports_common/filemanager.py
Outdated
Show resolved
Hide resolved
| formatted_error = "".join( | ||
| [f"\n{_SEP}{k}{_SEP}\n{v}" for k, v in fields.items()] | ||
| ) | ||
| logger.debug("Error serialized to client:%s", formatted_error) |
There was a problem hiding this comment.
it is just a matter of consistency actually. it will also prevent us from having to manage both text/json cases when parsing the responses
| class RCloneSettings(BaseCustomSettings): | ||
| R_CLONE_S3: S3Settings = Field(auto_default_from_env=True) | ||
| R_CLONE_PROVIDER: S3Provider | ||
| R_CLONE_REGION: str = Field("us-east-1", description="S3 region to use") |
There was a problem hiding this comment.
whether the S3_REGION is used or not is up to the client code. so I do not think this should create any problem.
| S3_SECURE: bool = False | ||
|
|
||
| @cached_property | ||
| def endpoint(self) -> str: |
There was a problem hiding this comment.
pair review comment: check if still required
There was a problem hiding this comment.
moved this to a validator
There was a problem hiding this comment.
Spent too much time on this. Requires much more time to refactor and make working with validator. Will leave as is.
…simcore-forked into remove-5gb-limit-nodeports
|
Kudos, SonarCloud Quality Gate passed!
|
@mrnicegyu11 @Surfict
Environment variables changes:
R_CLONE_S3_PROVIDERtoR_CLONE_PROVIDERdirector-v2(make sure they exist on the deployment):What do these changes do?
To avoid hitting a wall when uploading more than 5GB via AWS S3
rcloneis used to upload data to ports.nodeports_v2is provided with anr_clone_settingsinstance it will usercloneto upload data to s3s3_linkused by r_clone is generated bystoragestorageis called (using the same code as the previous method)rcloneupload fails, metadata entries will be removed (trying to reduce trash)dynamic-sidecarnow has a mandatoryr_clone_settingsfields.BONUS:
docker-compose up)Related issue/s
How to test
jupyter-math:2.0.5service.dynamic-sidecaryou will seer_clonebeing usedstorageservice calls to the metadata endpoint will be issued to:GET v0/locations/0/files/{s3_object}/s3/linkandPATCH v0/locations/0/files/_S3_OBJECT_/metadataChecklist
make openapi-specs,git commit ...and thenmake version-*)