feat: add polybox storage #527
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andre-code
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
|
You can access the deployment of this PR at https://renku-ci-ds-527.dev.renku.ch |
andre-code
force-pushed
the
andrea/3392-add-polybox-switchdrive-schema
branch
from
bd555db to
f567310
Compare
andre-code
force-pushed
the
andrea/3392-add-polybox-switchdrive-schema
branch
from
f567310 to
2896ebc
Compare
andre-code
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
andre-code
force-pushed
the
andrea/3392-add-polybox-switchdrive-schema
branch
from
2896ebc to
aadc15c
Compare
andre-code
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
andre-code
force-pushed
the
andrea/3392-add-polybox-switchdrive-schema
branch
from
aadc15c to
df5e967
Compare
andre-code
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
andre-code
force-pushed
the
andrea/3392-add-polybox-switchdrive-schema
branch
from
df5e967 to
df14d60
Compare
andre-code
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
andre-code
force-pushed
the
andrea/3392-add-polybox-switchdrive-schema
branch
from
df14d60 to
f0e351d
Compare
andre-code
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
andre-code
force-pushed
the
andrea/3392-add-polybox-switchdrive-schema
branch
from
f0e351d to
dfa1925
Compare
andre-code
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
andre-code
force-pushed
the
andrea/3392-add-polybox-switchdrive-schema
branch
from
dfa1925 to
e6dd8cf
Compare
andre-code
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
andre-code
force-pushed
the
andrea/3392-add-polybox-switchdrive-schema
branch
from
e6dd8cf to
83b1acb
Compare
andre-code
force-pushed
the
andrea/3392-add-polybox-switchdrive-schema
branch
from
9a4b60d to
207b3d4
Compare
andre-code
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
andre-code
force-pushed
the
andrea/3392-add-polybox-switchdrive-schema
branch
from
207b3d4 to
9c4f0a7
Compare
andre-code
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
andre-code
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
andre-code
force-pushed
the
andrea/3392-add-polybox-switchdrive-schema
branch
from
f4a6f1d to
26c6e3c
Compare
andre-code
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
andre-code
force-pushed
the
andrea/3392-add-polybox-switchdrive-schema
branch
from
26c6e3c to
578240d
Compare
Panaetius
requested changes
Nov 25, 2024
Comment on lines
230
to
235
| if access == "shared" and storage_type == "switchDrive": | ||
| self.configuration["url"] = "https://drive.switch.ch/public.php/webdav/" | ||
| if access == "personal" and storage_type == "polybox": | ||
| self.configuration["url"] = "https://polybox.ethz.ch/remote.php/webdav/" | ||
| if access == "personal" and storage_type == "switchDrive": | ||
| self.configuration["url"] = "https://drive.switch.ch/remote.php/webdav/" |
There was a problem hiding this comment.
nitpick: these should be elif since they're mutually exclusive.
| @@ -156,6 +157,91 @@ def __patch_schema_remove_oauth_propeties(spec: list[dict[str, Any]]) -> None: | |||
| options.append(option) | |||
| storage["Options"] = options | |||
|
|
|||
| @staticmethod | |||
| def __find_webdav_storage(spec: list[dict[str, Any]]) -> dict[str, Any]: | |||
There was a problem hiding this comment.
issue: this is too specific to me, I think having _find_storage(prefix: str) would be cleaner and there might be some other places where we could use that as well,e.g. in __patch_schema_add_switch_provider
| "IsPassword": False, | ||
| "NoPrefix": False, | ||
| "Advanced": False, | ||
| "Exclusive": False, |
There was a problem hiding this comment.
issue: Exclusive should be True, this means that only the values in Examples are valid values
There was a problem hiding this comment.
ok, thanks for clarify that
| "Provider": "", | ||
| }, | ||
| ], | ||
| "Required": False, |
There was a problem hiding this comment.
question: this field is mandatory, no? Especially since it doesn't have a default set
|
|
||
| custom_options = [ | ||
| { | ||
| "Name": "access", |
There was a problem hiding this comment.
question: I'm a bit confused by this, this is a field called access, but lower down its values are used as provider. In rclone schemas, the providers should be in the field provider by convention.
There was a problem hiding this comment.
Oh, I see where the confusion came from. I was trying to make it flexible by adding an option for access (personal and shared), but it ended up being a bit unclear. I’ll change it so the modes are set in the provider options instead, and I’ll use the provider property to specify when an option applies to a specific mode (personal or shared).
Panaetius
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
- make __find_webdav_storage as more reusable function - use provider option for set the personal and shared modes instead of creating an option for it - clean up code
andre-code
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
Panaetius
reviewed
Nov 25, 2024
| @@ -176,36 +176,10 @@ def __add_webdav_based_storage( | |||
| ) -> None: | |||
| """Create a modified copy of WebDAV storage and add it to the schema.""" | |||
| # Find WebDAV storage schema and create a modified copy | |||
| storage_copy = RCloneValidator.__find_webdav_storage(spec) | |||
| storage_copy = RCloneValidator.__find_storage(spec, "webDav") | |||
There was a problem hiding this comment.
Suggested change
| storage_copy = RCloneValidator.__find_storage(spec, "webDav") | |
| storage_copy = RCloneValidator.__find_storage(spec, "webdav") |
I think this is a typo? at least it was lower case before.
There was a problem hiding this comment.
yes, it is. thanks!
andre-code
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
andre-code
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
andre-code
force-pushed
the
andrea/3392-add-polybox-switchdrive-schema
branch
from
eaad207 to
9a58d06
Compare
andre-code
had a problem deploying
to
renku-ci-ds-527
GitHub Actions
Failure
andre-code
temporarily deployed
to
renku-ci-ds-527
GitHub Actions
Inactive
Panaetius
approved these changes
Dec 2, 2024
andre-code
temporarily deployed
to
renku-ci-ds-527
GitHub Actions
Inactive
Panaetius
reviewed
Dec 3, 2024
Comment on lines
+219
to
+242
|
|
||
| # Transform configuration for polybox or switchDrive | ||
| storage_type = self.configuration.get("type", "") | ||
| access = self.configuration.get("provider", "") | ||
|
|
||
| if storage_type == "polybox" or storage_type == "switchDrive": | ||
| self.configuration["type"] = "webdav" | ||
| self.configuration["provider"] = "" | ||
|
|
||
| if access == "shared" and storage_type == "polybox": | ||
| self.configuration["url"] = "https://polybox.ethz.ch/public.php/webdav/" | ||
| elif access == "shared" and storage_type == "switchDrive": | ||
| self.configuration["url"] = "https://drive.switch.ch/public.php/webdav/" | ||
| elif access == "personal" and storage_type == "polybox": | ||
| self.configuration["url"] = "https://polybox.ethz.ch/remote.php/webdav/" | ||
| elif access == "personal" and storage_type == "switchDrive": | ||
| self.configuration["url"] = "https://drive.switch.ch/remote.php/webdav/" | ||
|
|
||
| # Extract the user from the public link | ||
| if access == "shared" and storage_type in {"polybox", "switchDrive"}: | ||
| public_link = self.configuration.get("public_link", "") | ||
| user_identifier = public_link.split("/")[-1] | ||
| self.configuration["user"] = user_identifier | ||
|
|
There was a problem hiding this comment.
just as a reminder, these changes also need to be backported to https://github.com/SwissDataScienceCenter/renku-notebooks/blob/master/renku_notebooks/api/schemas/cloud_storage.py#L155 to work with Renku v1.
|
Tearing down the temporary RenkuLab deplyoment for this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR to modify cloud storage schema to add polybox storage
/deploy