Skip to content
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

fix: add vendor for polybox storage #604

Merged
merged 4 commits into from
Jan 20, 2025

Conversation

olevski
Copy link
Member

@olevski olevski commented Jan 13, 2025

This is meant to fix the issue here: https://www.notion.so/renku/Prevent-File-changed-pop-ups-for-Polybox-folders-1730df2efafc80948abee9f248cbef9b?pvs=4

It is a popup that shows up every time you mount a polybox storage and you edit a file:
image (19)

There may be a few other options we can cleanup in the csi-rclone code. But this one is the main one that is causing the issue.

/deploy renku=release-0.64.0

@olevski olevski requested a review from a team as a code owner January 13, 2025 16:06
@olevski olevski marked this pull request as draft January 13, 2025 16:06
@olevski olevski temporarily deployed to renku-ci-ds-604 January 13, 2025 16:06 — with GitHub Actions Inactive
@RenkuBot
Copy link
Contributor

You can access the deployment of this PR at https://renku-ci-ds-604.dev.renku.ch

@coveralls
Copy link

coveralls commented Jan 13, 2025

Pull Request Test Coverage Report for Build 12851284222

Details

  • 0 of 1 (0.0%) changed or added relevant line in 1 file are covered.
  • 2 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.001%) to 86.897%

Changes Missing Coverage Covered Lines Changed/Added Lines %
components/renku_data_services/notebooks/api/schemas/cloud_storage.py 0 1 0.0%
Files with Coverage Reduction New Missed Lines %
components/renku_data_services/base_api/error_handler.py 1 89.47%
components/renku_data_services/crc/blueprints.py 1 93.06%
Totals Coverage Status
Change from base Build 12792204967: 0.001%
Covered Lines: 15246
Relevant Lines: 17545

💛 - Coveralls

@olevski
Copy link
Member Author

olevski commented Jan 14, 2025

This is currently blocked by the UI and how it interprets fields in the rclone config and which of these fields are sent to the backend when a data connector is created.

Besically it seems that there is no way to have a field in the rclone config that has a default and is also sent to the backend when a cloud storage is created. If we want the ui to send the field to the backend then the field should be set to required without a default value.

@lorenzo-cavazzi correctly if I am wrong here. But this is my understanding from our quick discussion.

EDIT: I figured it out. We set the vendor now whenever we create the config for the rclone csi pvc when we launch a session. And this is only for next cloud and switch drive.

@olevski olevski force-pushed the fix-add-vendor-field-for-polybox-webdav-provider branch from c90f744 to e684d3f Compare January 15, 2025 13:32
@olevski olevski temporarily deployed to renku-ci-ds-604 January 15, 2025 13:33 — with GitHub Actions Inactive
@olevski olevski temporarily deployed to renku-ci-ds-604 January 15, 2025 13:46 — with GitHub Actions Inactive
@olevski olevski force-pushed the fix-add-vendor-field-for-polybox-webdav-provider branch from e684d3f to ea60502 Compare January 15, 2025 16:32
@olevski olevski temporarily deployed to renku-ci-ds-604 January 15, 2025 16:33 — with GitHub Actions Inactive
@olevski olevski marked this pull request as ready for review January 16, 2025 10:07
@olevski olevski force-pushed the fix-add-vendor-field-for-polybox-webdav-provider branch from 55ab8fe to 44dd94d Compare January 16, 2025 10:08
@olevski olevski temporarily deployed to renku-ci-ds-604 January 16, 2025 10:08 — with GitHub Actions Inactive
@leafty
Copy link
Member

leafty commented Jan 16, 2025

@olevski I think you may want to deploy against renku=release-0.64.0 so that you include the latest UI fixes for data connectors.

@olevski olevski temporarily deployed to renku-ci-ds-604 January 16, 2025 14:10 — with GitHub Actions Inactive
leafty
leafty previously approved these changes Jan 16, 2025
Copy link
Member

@leafty leafty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix seems to work for polybox storage 👍 👏

@olevski olevski merged commit e27a7d0 into main Jan 20, 2025
17 checks passed
@olevski olevski deleted the fix-add-vendor-field-for-polybox-webdav-provider branch January 20, 2025 08:12
@RenkuBot
Copy link
Contributor

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants