nextcloud: support several base paths#3370
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds automatic detection and persistence of a working Nextcloud WebDAV base path so Nextcloud backups can be uploaded against different common endpoint layouts, and bumps datastore to introduce the new configuration field.
Changes:
- Extend Nextcloud backup-cloud configuration with an optional
base_path. - Try several WebDAV base-path candidates on upload, persist the first working one back into the system config.
- Add datastore migration
123to ensurebase_pathexists for existing Nextcloud configurations.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/modules/backup_clouds/nextcloud/config.py | Adds base_path to the Nextcloud configuration object. |
| packages/modules/backup_clouds/nextcloud/backup_cloud.py | Implements base-path probing on upload and uses the stored base path for retention operations. |
| packages/helpermodules/update_config.py | Bumps DATASTORE_VERSION and adds migration step 123 for Nextcloud backup-cloud config. |
Comments suppressed due to low confidence (1)
packages/modules/backup_clouds/nextcloud/backup_cloud.py:19
- Die Docstring-Beschreibung passt nicht mehr zur Signatur/Return-Value: Es wird hier kein WebDAV-Pfad mehr zurückgegeben (nur upload_url und user). Bitte Docstring aktualisieren, damit er das aktuelle Verhalten korrekt beschreibt.
def _parse_nextcloud_upload_url_and_user(config: NextcloudBackupCloudConfiguration) -> Tuple[str, str]:
"""
Liefert Basis-URL (ohne /public.php/webdav/...) und Benutzer (Token oder User).
Zusätzlich wird der WebDAV-Pfad zum Backup-Verzeichnis zurückgegeben.
"""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+159
to
+165
|
|
||
| for base_path in [config.configuration.base_path] + base_path_candidates: | ||
| try: | ||
| if base_path: | ||
| upload_call(base_path) | ||
| config.configuration.base_path = base_path | ||
| Pub().pub("openWB/set/system/backup_cloud/config", asdict(config)) |
|
|
||
| def upgrade_datastore_123(self) -> None: | ||
| def upgrade(topic: str, payload) -> Optional[dict]: | ||
| if re.search("openWB/system/backup_cloud/config", topic) is not None: |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
packages/modules/backup_clouds/nextcloud/backup_cloud.py:19
- The docstring is now inaccurate:
_parse_nextcloud_upload_url_and_useronly returns(upload_url, user), but the comment still says it also returns the WebDAV path. Please update the docstring to match the current return values/behavior to avoid misleading future changes.
def _parse_nextcloud_upload_url_and_user(config: NextcloudBackupCloudConfiguration) -> Tuple[str, str]:
"""
Liefert Basis-URL (ohne /public.php/webdav/...) und Benutzer (Token oder User).
Zusätzlich wird der WebDAV-Pfad zum Backup-Verzeichnis zurückgegeben.
"""
Comment on lines
+137
to
144
| def _put_backup_file(upload_url: str, | ||
| base_path: str, | ||
| backup_filename: str, | ||
| backup_file: bytes, | ||
| user: str, | ||
| password: str) -> None: | ||
| req.get_http_session().put( | ||
| f"{upload_url}{base_path}/{backup_filename.lstrip('/')}", |
Comment on lines
+152
to
+167
| def _determine_working_base_path(config: NextcloudBackupCloud, | ||
| user: str, | ||
| upload_call: Callable[[str], None]) -> None: | ||
| base_path_candidates = ["/public.php/webdav", | ||
| f"/public.php/dav/files/{user}", | ||
| "/remote.php/webdav", | ||
| f"/remote.php/dav/files/{user}"] | ||
|
|
||
| for base_path in [config.configuration.base_path] + base_path_candidates: | ||
| try: | ||
| if base_path: | ||
| upload_call(base_path) | ||
| if base_path != config.configuration.base_path: | ||
| config.configuration.base_path = base_path | ||
| Pub().pub("openWB/set/system/backup_cloud/config", asdict(config)) | ||
| return |
Comment on lines
+3115
to
+3118
| if (configuration_payload.get("type") == "nextcloud" and | ||
| configuration_payload["configuration"].get("base_path") is None): | ||
| configuration_payload["configuration"].update({"base_path": None}) | ||
| return {topic: configuration_payload} |
This file contains hidden or 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
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.
No description provided.