Driver google drive #35
Conversation
| @@ -28,7 +29,7 @@ def validate_config(config): | |||
| ): | |||
| raise ConfigError("Config error: Incorrect mergin settings") | |||
|
|
|||
| if config.driver not in ["local", "minio"]: | |||
| if config.driver not in ["local", "minio", "google_drive"]: | |||
There was a problem hiding this comment.
I'd suggest moving to enum (of supported drivers) rather than expanding list with strings, all comparisons in code would be less error prone.
| @@ -66,6 +67,13 @@ def validate_config(config): | |||
| ): | |||
| raise ConfigError("Config error: Incorrect media reference settings") | |||
|
|
|||
| if config.driver == "google_drive" and not ( | |||
There was a problem hiding this comment.
I wonder if we should not move these kinds of validations to driver class (e.g. validate() or even to init)
| @@ -81,3 +89,14 @@ def update_config_path( | |||
| config.update(user_file_config) | |||
| else: | |||
| raise IOError(f"Config file {config_file_path} does not exist.") | |||
|
|
|||
|
|
|||
| def get_share_with(google_driver_settings) -> typing.List[str]: | |||
There was a problem hiding this comment.
is not this more like config validation to be used in check above? Or maybe split between check and getter in driver?
| self._folder_id = self._create_folder(self._folder) | ||
|
|
||
| for email in get_share_with(config.google_drive): | ||
| if email: |
There was a problem hiding this comment.
it would be better if get_share_with returned only list of valid emails or empty list
|
|
||
| file_id = file.get("id") | ||
|
|
||
| except Exception as e: |
There was a problem hiding this comment.
Is there a reason for such broad exception? I can see in many places call of some .execute() method which might have a well defined exception?
| ) | ||
|
|
||
| # Check if email exists in permissions | ||
| for permission in permissions.get("permissions", []): |
There was a problem hiding this comment.
maybe use next(condition, False) ?
| "DRIVER": "google_drive", | ||
| "GOOGLE_DRIVE__SERVICE_ACCOUNT_FILE": GOOGLE_DRIVE_SERVICE_ACCOUNT_FILE, | ||
| "GOOGLE_DRIVE__FOLDER": GOOGLE_DRIVE_FOLDER, | ||
| "GOOGLE_DRIVE__SHARE_WITH": "jan.caha@lutraconsulting.co.uk", |
There was a problem hiding this comment.
not sure it is good to have your email here for automated tests
| prepare_mergin_project(mc, full_project_name) | ||
|
|
||
| # patch config to fit testing purposes | ||
| config.update( |
There was a problem hiding this comment.
can you add some tests for invalid config?
|
|
||
| google_drive_delete_folder(GoogleDriveDriver(config), GOOGLE_DRIVE_FOLDER) | ||
|
|
||
| main() |
There was a problem hiding this comment.
There is quite a few extra methods in driver, I wonder if we should not test also some basic failures, even with mocked api or something.
| # patch config to fit testing purposes | ||
| config.update( | ||
| { | ||
| "OPERATION_MODE": "move", |
There was a problem hiding this comment.
Do we need to test both, move and copy modes, is not is driver agnostic?
Fixes #13