Skip to content

Conversation

@moonyuet
Copy link
Member

@moonyuet moonyuet commented Jan 7, 2026

Changelog Description

This PR is to add six into module blacklist for representation trait so that loading the asset with the trait won't hit the error with the six module.

File "D:\ayon_launcher\ayon-launcher\build\output\dependencies\urllib3\packages\six.py", line 96, in get
    result = self._resolve()
             ^^^^^^^^^^^^^^^
  File "D:\ayon_launcher\ayon-launcher\build\output\dependencies\urllib3\packages\six.py", line 118, in _resolve
    return _import_module(self.mod)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\ayon_launcher\ayon-launcher\build\output\dependencies\urllib3\packages\six.py", line 87, in _import_module
    import(name)
  File "C:\Users/Public/Documents/MarvelousDesigner/Configuration/python311/Lib\dbm\gnu.py", line 3, in <module>
    from _gdbm import *
ModuleNotFoundError: No module named '_gdbm'

Additional info

Discover the issue when testing ynput/ayon-marvelous-designer#2

Testing notes:

  1. Build Core addon with this branch
  2. Load asset with the representation trait.

@moonyuet moonyuet requested a review from antirotor January 7, 2026 06:00
@moonyuet moonyuet self-assigned this Jan 7, 2026
@moonyuet moonyuet added the type: enhancement Improvement of existing functionality or minor addition label Jan 7, 2026
@ynbot ynbot added the size/XS label Jan 7, 2026
@BigRoy
Copy link
Collaborator

BigRoy commented Jan 7, 2026

won't hit the error with the six module.

I see no linked issue. What error?

@moonyuet
Copy link
Member Author

moonyuet commented Jan 7, 2026

won't hit the error with the six module.

I see no linked issue. What error?

See the changelog.

@BigRoy
Copy link
Collaborator

BigRoy commented Jan 7, 2026

Is that the full traceback, or is it supposed to be longer? I'd have expected it to point further with the traceback to what code of ours generated the error.


It feels weird that our logic even touches the six package, or in this case even the urllib3 package? But it seems we're just searching all sys.modules which seems like a rather heavy-weight solution. @antirotor could you elaborate why we didn't opt for a dedicated traits plug-in path like how the rest of AYON works to discover particular entries? Somehow I feel like we should be telling it where to look, instead of telling it what modules to exclude?

Also, this str.startswith will incorrectly allow e.g. id my.first.hello to also match my.first.helloworld which it shouldn't?
And, there's quite some lru caching there. But I feel like if there's something to cache - it's mostly the inspecting of the classes across all the modules, regardless of trait id. E.g. I'd cache the output of this: https://github.com/ynput/ayon-core/blob/6874032f767116d87ac10fd157f0eac6f86570a8/client/ayon_core/pipeline/traits/representation.py#L486C9-L508 which would only really need a single lru_cache entry anyway, since it's not dynamic.

Then all you'd need after is iterate these traits to find them by id? Instead of having to go over sys.modules again for all trait ids that match your query?

@antirotor
Copy link
Member

@antirotor could you elaborate why we didn't opt for a dedicated traits plug-in path like how the rest of AYON works to discover particular entries? Somehow I feel like we should be telling it where to look, instead of telling it what modules to exclude?

The idea behind this is that you can work with traits as with types. You can, ofc change it so every time you invoke trait related logic, you run "discover()" and interate over the result somehow. But I wanted to give it more pythonic feel - you import whatever you need and work with it. It is IMO more explicit than calling some discover function (that would have to import traits to global scope to make them available anyway).

This logic is used only when you are deserializing traits from json (or something else) - at that point, you don't know what is what and the function is mapping trait ids to their classes. We could hold the traits in some smaller scope and look just there for example.

Once the traits could be used by server, we will need to revisit this somehow anyway - we will need some mechanism for every addon to expose it's own representation traits via lightweight API so server could use it too.

So far, it works with minor tweaks. With a little feedback I've got I am glad for every oportunity to discuss and change any decisions made.

In this very specific case I think the issue is more about six still being in use and I hope that with the move to python 3.11 this will disappear although there are chances that more modules will cause issues and that would be good point to start thinking about refactoring it.

@BigRoy
Copy link
Collaborator

BigRoy commented Jan 7, 2026

In this very specific case I think the issue is more about six still being in use and I hope that with the move to python 3.11 this will disappear although there are chances that more modules will cause issues and that would be good point to start thinking about refactoring it.

This was my biggest worry. Thanks.

@moonyuet moonyuet merged commit 411a815 into develop Jan 7, 2026
16 checks passed
@moonyuet moonyuet deleted the enhancement/add_six_asmodule_blacklist_for_representation_trait branch January 7, 2026 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS type: enhancement Improvement of existing functionality or minor addition

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants