Proposal to allow custom Descriptors via hooks

[Ahuge]

This PR is to allow toolkit developers to write custom IODescriptor classes without needing to take ownership over the entire tk-core repository.

We are no longer relying on a function in the __init__.py of tank.descriptor.io_descriptor to register our Descriptor types.
We have offloaded that work to the IODescriptorBase create method on it's first execution.

We have a class level initializer check that will call the external hook and fall back to the default types.

I have also added a sample use case for implementing a bitbucket_release IODescriptor example based off of the github_release IODescriptor

[Ahuge]

I don't think the test errors are me.. I see the PRs right before this have the same errors

[Ahuge]

Any chance I could get someone to look at this?

@pscadding @eshokrgozar

Would love to get thoughts on it...

Thanks

[jfboismenu]

Hi @Ahuge ,

We've been interested in allowing people to add their own custom io descriptor types over the years, but we've never had a chance to prioritize it. While we understand the need to add new IODescriptor types, we're curious why you would need a custom Descriptor type. Could you elaborate on this concept?

With that being said, it's great that you've put so much effort in this submission, it's well documented, but we can't accept it at the moment.

There's the matter that it only works when using the ToolkitManager. Not every Toolkit workflow use the ToolkitManager. For example, many workflows involving creating a Toolkit instance using sgtk.sgtk_from_path and then start the engine manually through sgtk.platform.start_engine. The fact that the descriptor registries are only initialized when using the ToolkitManager will cause many problems. You can see this in the many tests that are now failing due to Unknown descriptor type for ...-like errors. For this reason, it seems this isn't a job for the ToolkitManager. If you want to run the tk-core unit tests, simply install tk-toolchain and run pytest at the root of the repository.

Also, I'd like to argue that custom descriptors should probably be available before bootstrapping, so it can be used in the descriptor field on a Pipeline Configuration entity in Shotgun. For example, might want your pipeline configuration to point to a specific bitbucket release.

We'd love to continue this with you or any other client (I know some have forked tk-core to have a tk-rez descriptor), so don't consider this as a refusal to see the feature be introduced at some point. Maybe you can start this in the community forum?

Let us know what you think!

JF

[Ahuge]

Hey @jfboismenu,

I appreciate the reply!
I will continue working on this one. Thanks for the tip about tk-toolkit I'll make sure I am doing that.

And in response to the Descriptor question, I think I was just adding it because I was in the area and someone someday might want descriptor customization maybe?
I can remove it from this PR if you'd prefer, because I personally don't need it. I just need the IODescriptor part.

Thanks
-Alex

[jfboismenu]

Hi again!

Yeah, we would err on the side of caution and not allow custom Descriptor derived classes for now. Custom IODescriptors is what people seem to be asking for right now.

We want to be honest. We don't want to tell you not to work on your own implementation of this for your studio. You're obviously the best person to decide whether maintaining a fork of tk-core for your studio is best. However, given that we do not have currently the bandwidth to fully look at the impact on Toolkit of such a change, it's possible that where you end up on your own might be different than where we would end up if we ever implement this feature ourselves.

You may find it best for now to simply live with your fork of tk-core in the state it is in. I think starting the discussion with the community, seeing the appetite for it and the needs of our clients is the best first step. The last thing we want is to send you on a wild goose chase trying to think of every impact this feature can have on Toolkit.

If you haven't started a thread yet on the forum, let me know and I'll start one.

Also, may I ask, which problem are you trying to get around here specifically by having a custom descriptor?

JF

[Ahuge]

https://community.shotgunsoftware.com/t/custom-iodescriptor-types-for-tk-core/8806

[Ahuge]

Is this me?

Starting: Cloning UI automation tools
==============================================================================
Task         : Bash
Description  : Run a Bash script on macOS, Linux, or Windows
Version      : 3.163.2
Author       : Microsoft Corporation
Help         : https://docs.microsoft.com/azure/devops/pipelines/tasks/utility/bash
==============================================================================
Generating script.
Script contents:
git clone --depth 1 [email protected]:$UI_AUTOMATION_REPO ../ui_automation
========================== Starting Command Output ===========================
/bin/bash --noprofile --norc /home/vsts/work/_temp/faedb517-3acb-485e-a59a-7b6890fc91c0.sh
Cloning into '../ui_automation'...
Warning: Permanently added the RSA host key for IP address '140.82.118.3' to the list of known hosts.
[email protected]: Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

[Ahuge]

Built docs

[Ahuge]

Built docs

Couple of typos there I need to fix

[hound]

local variable 'd' is assigned to but never used

[jfboismenu]

Re: Is this me?

No, we made a mistake in our build pipeline. This step should not be executed from fork, as forks do not have access to the build pipeline secrets.

[Ahuge]

Oh perfect good to hear haha!

That makes sense thanks!