URI repository paths #1956

tokejepsen · 2021-08-23T14:56:33Z

Goal

Support URI paths like file:// and dropbox://.

Closes #1908

Motivation

For some of our remote users we cant ensure access to local repository paths, so we have a need for storing our versions in the cloud.

Steps to test
Dropbox is the only supported cloud storage atm. For Dropbox you'll need to create an app: https://developers.dropbox.com/oauth-guide

I opted to use an app because you can have a separate root folder for the app, which makes it easier to organize.

Input the access token in the settings Dropbox section.

Implementation

I initially tried to do custom implementations of Path like objects, but quickly found that the library https://cloudpathlib.drivendata.org/ had already figured things out, so I opted to use that.
The library does not however support Dropbox (which we need), so there is a custom class for Dropbox. That same goes for the uri file:// which is just a wrapper for the built-in Path object.
The AnyPath wrapper determines which class to use depending on the uri and if its a local path it'll revert to built-in Path object. This means we can use the AnyPath as a replacement for Path everywhere.

I've initially put its all in tools but it might need a separate module.

There is a caching system in cloudpath that'll localize the data when reading it. In order for this to work smoothly with the user data directory, I'm using the user data directory as the cache directory. To optimize download I changed to keeping the zip file after extraction. To further optimize download I've changed the validation to happen after the versions have been found, else we would be downloading zip files just validate against the version.py inside.
To keep the user data directory clean, we delete any zip file that is not the current and any directory. Let me know if there is any issues with this, that you can think of.

The only thing I dont like about this implementation is the hack to get the system settings inside of the Dropbox class. This is because when we get the repository paths and validate their existence, we need the dropbox token.

[cuID:yjx9fd]

- lock requests to 2.25 - add cloudpathlib - add dropbox

- AnyPath falls back to Path when recognizing a local path. - custom FilePath for "file://" handling. - custom DropboxPath for "dropbox://" handling.

- and cache to local user data directory.

Needed for start.py is_inside check. Just returns the local cached path.

This reverts commit 895cfd8.

If a latest version is valid we return that rather than validating all versions.

# Conflicts: # openpype/version.py

mkolar · 2021-09-01T07:39:36Z

resolves #1908

tokejepsen · 2021-09-02T09:39:41Z

We could potentially unify the setup process with #1979, by using Team Folders, so people only have to make one Dropbox app, and one place for all the settings.

antirotor

I've just found one unpleasant side-effect of this - if you enable dropbox module, set your token and then enter path to OpenPype repositories to dropbox://... and it will fail for some reason, you can't run settings anymore (anything from openpype in fact) as get_openpype_path_from_db() is called early in bootstrap process and it will pull in dropbox to check if path exists.

Some solution is with changing order how path is determined here:
https://github.com/pypeclub/OpenPype/blob/2a01fb8f3165b237161f2ad6877d9a6213c79087/start.py#L811

so check for OPENPYPE_PATH should be before this, but nevertheless we should probably handle dropbox errors more gracefully.

tokejepsen · 2021-09-07T06:54:16Z

but nevertheless we should probably handle dropbox errors more gracefully.

Do you have any stack trace of the errors?

antirotor · 2021-09-08T09:54:08Z

Do you have any stack trace of the errors?

yeah, sorry:

Traceback (most recent call last):
  File "C:\Users\annat\Documents\pype\3.0\start.py", line 940, in <module>
    boot()
  File "C:\Users\annat\Documents\pype\3.0\start.py", line 786, in boot
    openpype_path = get_openpype_path_from_db(openpype_mongo)
  File "C:\Users\annat\Documents\pype\3.0\igniter\tools.py", line 590, in get_openpype_path_from_db
    if AnyPath(path).exists():
  File "C:\Users\annat\Documents\pype\3.0\.venv\lib\site-packages\cloudpathlib\cloudpath.py", line 302, in exists
    return self.client._exists(self)
  File "C:\Users\annat\Documents\pype\3.0\igniter\tools.py", line 278, in _exists
    self.client.files_get_metadata(cloud_path.path)
  File "C:\Users\annat\Documents\pype\3.0\.venv\lib\site-packages\dropbox\base.py", line 1512, in files_get_metadata
    None,
  File "C:\Users\annat\Documents\pype\3.0\.venv\lib\site-packages\dropbox\dropbox_client.py", line 329, in request
    timeout=timeout)
  File "C:\Users\annat\Documents\pype\3.0\.venv\lib\site-packages\dropbox\dropbox_client.py", line 485, in request_json_string_with_retry
    timeout=timeout)
  File "C:\Users\annat\Documents\pype\3.0\.venv\lib\site-packages\dropbox\dropbox_client.py", line 611, in request_json_string
    raise AuthError(request_id, err)
dropbox.exceptions.AuthError: AuthError('f541561c6e5e4c7819a01325dfa470ce4', AuthError('missing_scope', TokenScopeError(required_scope='files.metadata.read')))

but that error is not really relevant. I've added files.metadata.read permissions and it still throws it so my issue is probably elsewhere, but point is that if there is any error at all from dropbox point of view, it will crash openpype at the early stages.

tokejepsen · 2021-09-08T10:41:31Z

Good point! We'll need to gracefully authenticate Dropbox else use the local path I guess?

antirotor · 2021-09-08T10:52:41Z

Good point! We'll need to gracefully authenticate Dropbox else use the local path I guess?

Maybe it would be enough to catch exception in:

if AnyPath(path).exists():
   ...

because you can say that if exists() fails, path doesn't exist :D

tokejepsen · 2021-09-08T11:42:20Z

True, we could do a try catch all.

And fallback on the local user directory?

antirotor · 2021-09-08T11:55:20Z

And fallback on the local user directory?

Yeah, issue warning and fallback to what can be found locally. That warning should be in UI if not in headless mode so artist would know exactly that he's not using "official" studio version but possibly outdated one and he need to contact someone to fix it.

tokejepsen · 2021-09-27T10:33:12Z

Note to self. Something that keeps tripping me up when working with the Dropbox API is that the permissions are attached to the token. So if you update the permissions you'll need to update the token as well.

# Conflicts: # igniter/tools.py # openpype/settings/entities/schemas/system_schema/schema_modules.json

tokejepsen · 2021-09-27T11:32:45Z

This is ready for another review.

# Conflicts: # igniter/tools.py # poetry.lock # pyproject.toml

antirotor

we are missing dependency in pyproject.toml:

cloudpathlib = "^0.6.2"

and we are missing documentation for this, but otherwise it seem to be working ok

antirotor · 2021-10-18T13:27:01Z

also let's remove the clean versions function and put it to different pull request so we can track it cleanly in changelog. This cleanup should be configurable on local settings level - because you actually might need multiple versions of OpenPype on render nodes - version si passed to Deadline as environment variable.

mkolar · 2021-10-25T15:10:58Z

@tokejepsen could you please alter this based on @antirotor last comment, this included two features that are not related and the one with version cleanup need a bit more love. Also needs to be separate for the changelog which is generated automatically

tokejepsen · 2021-10-25T15:16:42Z

Yup, this is on my list to do at some point.

iLLiCiTiT · 2022-02-10T12:58:01Z

igniter/tools.py

@@ -128,6 +428,39 @@ def validate_path_string(path: str) -> (bool, str):
 return True, "valid path"


+def get_openpype_system_settings(url: str) -> dict:


I'm afraid it is not possible to use system settings before defying which OpenPype is used. That's why we separated few settings into global settings get_openpype_global_settings. Main reason is that we knew that we will want to store settings by version (implemented with #2570).

iLLiCiTiT · 2022-02-10T12:59:35Z

igniter/tools.py

+
+# Hack to share url to AnyPath paths.
+MODULE = sys.modules[__name__]
+URL = ""


You should use os.environ["OPENPYPE_MONGO"] instead of storing the value into global scope variable. Or pass the value into function where is needed.

iLLiCiTiT · 2022-02-10T13:11:30Z

igniter/tools.py

@@ -14,6 +15,305 @@
 ConfigurationError,
 OperationFailure
 )
+from cloudpathlib import AnyPath


Code specific for dropbox should be in separated file and only code that is used here should be imported. There is a lot of code which is dropbox specific but does not make sense in context of igniter/tools for example class FilePath is dropbox specific but I this context it is confusing.

ClementHector · 2022-04-05T06:33:26Z

I will soon start implementing the same for Google Drive. Wouldn't the easiest way be to fork cloudpathlib and add the class for
dropbox and gdrive to it?

tokejepsen · 2022-04-05T07:38:56Z

I will soon start implementing the same for Google Drive. Wouldn't the easiest way be to fork cloudpathlib and add the class for
dropbox and gdrive to it?

Yeah, it might be. I just didnt have time to figure out where it should be. Also a fork might be more trouble managing with OP than a custom class, but forking might lead to a PR to origin repository which would be better.

mkolar · 2022-04-05T09:05:58Z

We had to actually put this on hold, until we finish refactor of the repository. To implement this cleanly it should be modules that provide the path cloud location, however those are not loaded until we start OP. there's a bit of an chicken and egg situation. We're counting on separating these responsibilities properly soon.

mkolar · 2022-04-28T17:25:47Z

Unfortunatelly, we won't be able to merge this in this state. It is however one of the first things to tackle as soon as we're done with the refactor of the client application, which is being changed to allow for this to be re-implemented a lot more robustly, without having to hardcode any specific providers into the product.

tokejepsen added 25 commits August 18, 2021 16:59

Update environment

05f6dee

- lock requests to 2.25 - add cloudpathlib - add dropbox

Replace Path with AnyPath

3a94f46

- AnyPath falls back to Path when recognizing a local path. - custom FilePath for "file://" handling. - custom DropboxPath for "dropbox://" handling.

Move FilePath and DropboxPath to tools.

3b27530

Move user_data_dir to tools.

d1bf208

Get dropbox token from settings

0ffebe5

- and cache to local user data directory.

Update version

8e37cf3

Update version

e460c4b

Add resolve

da1937d

Needed for start.py is_inside check. Just returns the local cached path.

Increment version.

82634f1

Increment version

d9c9620

Keep zip file after extraction

895cfd8

Increment version

1e808a0

Revert "Keep zip file after extraction"

86daf9e

This reverts commit 895cfd8.

Dropbox cache as subdirectory.

459b533

Increment version

96d7df6

Expose clean_cache method.

75f0d6b

Clean cloud caches when cleaning after zip extraction.

1fdda70

Increment version

428bea3

Increment version

501667e

Clean cache with root path.

69d6cde

Clean repository when extracting a new version.

38954d2

Remove clean cache

504c43a

Reset dropbox cache to user data dir

54fe31a

Validate versions

29babd5

If a latest version is valid we return that rather than validating all versions.

Merge branch 'develop' into feature/uri_repository_path

ef26ea5

# Conflicts: # openpype/version.py

tokejepsen added the type: feature Larger, user affecting changes and completely new things label Aug 23, 2021

tokejepsen requested review from mkolar, kalisp, antirotor and iLLiCiTiT August 23, 2021 14:56

antirotor suggested changes Sep 6, 2021

View reviewed changes

tokejepsen added 3 commits September 27, 2021 11:50

Catch all expection for when checking existence.

db87a76

Merge branch 'develop' into feature/uri_repository_path

29dd42e

# Conflicts: # igniter/tools.py # openpype/settings/entities/schemas/system_schema/schema_modules.json

Issue warning to user when not paths exist.

a1e2a07

mkolar requested a review from antirotor September 29, 2021 11:22

tokejepsen added 2 commits October 7, 2021 14:59

Update for Mongo certifi issue.

842838c

Merge branch 'develop' into feature/uri_repository_path

5b8ed4b

# Conflicts: # igniter/tools.py # poetry.lock # pyproject.toml

antirotor suggested changes Oct 18, 2021

View reviewed changes

Merge branch 'develop' into feature/uri_repository_path

52d3f81

iLLiCiTiT reviewed Feb 10, 2022

View reviewed changes

mkolar closed this Apr 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

URI repository paths #1956

URI repository paths #1956

tokejepsen commented Aug 23, 2021 •

edited by ynbot

Loading

mkolar commented Sep 1, 2021 •

edited

Loading

tokejepsen commented Sep 2, 2021

antirotor left a comment

tokejepsen commented Sep 7, 2021

antirotor commented Sep 8, 2021 •

edited

Loading

tokejepsen commented Sep 8, 2021

antirotor commented Sep 8, 2021

tokejepsen commented Sep 8, 2021

antirotor commented Sep 8, 2021

tokejepsen commented Sep 27, 2021

tokejepsen commented Sep 27, 2021

antirotor left a comment

antirotor commented Oct 18, 2021

mkolar commented Oct 25, 2021

tokejepsen commented Oct 25, 2021

iLLiCiTiT Feb 10, 2022

iLLiCiTiT Feb 10, 2022

iLLiCiTiT Feb 10, 2022

ClementHector commented Apr 5, 2022 •

edited

Loading

tokejepsen commented Apr 5, 2022 •

edited

Loading

mkolar commented Apr 5, 2022

mkolar commented Apr 28, 2022

		@@ -128,6 +428,39 @@ def validate_path_string(path: str) -> (bool, str):
		return True, "valid path"


		def get_openpype_system_settings(url: str) -> dict:

URI repository paths #1956

URI repository paths #1956

Conversation

tokejepsen commented Aug 23, 2021 • edited by ynbot Loading

mkolar commented Sep 1, 2021 • edited Loading

tokejepsen commented Sep 2, 2021

antirotor left a comment

Choose a reason for hiding this comment

tokejepsen commented Sep 7, 2021

antirotor commented Sep 8, 2021 • edited Loading

tokejepsen commented Sep 8, 2021

antirotor commented Sep 8, 2021

tokejepsen commented Sep 8, 2021

antirotor commented Sep 8, 2021

tokejepsen commented Sep 27, 2021

tokejepsen commented Sep 27, 2021

antirotor left a comment

Choose a reason for hiding this comment

antirotor commented Oct 18, 2021

mkolar commented Oct 25, 2021

tokejepsen commented Oct 25, 2021

iLLiCiTiT Feb 10, 2022

Choose a reason for hiding this comment

iLLiCiTiT Feb 10, 2022

Choose a reason for hiding this comment

iLLiCiTiT Feb 10, 2022

Choose a reason for hiding this comment

ClementHector commented Apr 5, 2022 • edited Loading

tokejepsen commented Apr 5, 2022 • edited Loading

mkolar commented Apr 5, 2022

mkolar commented Apr 28, 2022

tokejepsen commented Aug 23, 2021 •

edited by ynbot

Loading

mkolar commented Sep 1, 2021 •

edited

Loading

antirotor commented Sep 8, 2021 •

edited

Loading

ClementHector commented Apr 5, 2022 •

edited

Loading

tokejepsen commented Apr 5, 2022 •

edited

Loading