Chore: Create plugin does not expect system settings #5553

iLLiCiTiT · 2023-09-01T15:06:44Z

Changelog Description

System settings are not passed to initialization of create plugin initialization (and apply_settings).

Additional info

Reasons: The system settings are not used anywhere, and since AYON there won't be 2 types of settings.

We need validation of all Create plugins: They should not use system_settings in apply_settings method and they should not override __init__ method.

To make this PR safer the system settings can still be passed to creator initialization, but eventually it will be removed in future PR anyway, so it would be just temporary backwards compatibility.

This PR must be updated with latest develop and revalidated changes before merge.

Testing notes:

Technically run all available creators in Publisher

hound

Some files could not be reviewed due to errors:

Traceback (most recent call last):

Traceback (most recent call last):
  File "/usr/local/bin/flake8", line 8, in 
    sys.exit(main())
  File "/usr/local/lib/python3.8/dist-packages/flake8/main/cli.py", line 18, in main
    app.run(argv)
  File "/usr/local/lib/python3.8/dist-packages/flake8/main/application.py", line 393, in run
    self._run(argv)
  File "/usr/local/lib/python3.8/dist-packages/flake8/main/application.py", line 381, in _run
    self.run_checks()
  File "/usr/local/lib/python3.8/dist-packages/flake8/main/application.py", line 300, in run_checks
    self.file_checker_manager.run()
  File "/usr/local/lib/python3.8/dist-packages/flake8/checker.py", line 331, in run
    self.run_serial()
  File "/usr/local/lib/python3.8/dist-packages/flake8/checker.py", line 315, in run_serial
    checker.run_checks()
  File "/usr/local/lib/python3.8/dist-packages/flake8/checker.py", line 598, in run_checks
    self.run_ast_checks()
  File "/usr/local/lib/python3.8/dist-packages/flake8/checker.py", line 502, in run_ast_checks
    for (line_number, offset, text, check) in runner:
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 56, in run
    parser.visit(self.tree)
  File "/usr/lib/python3.8/ast.py", line 371, in visit
    return visitor(node)
  File "/usr/lib/python3.8/ast.py", line 379, in generic_visit
    self.visit(item)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 39, in visit_ClassDef
    self.capture_issues_visitor('ClassDef', node)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 33, in capture_issues_visitor
    self.generic_visit(node)
  File "/usr/lib/python3.8/ast.py", line 381, in generic_visit
    self.visit(value)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 36, in visit_Call
    self.capture_issues_visitor('Call', node)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 30, in capture_issues_visitor
    issues = checker.run(node)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checkers/render.py", line 22, in run
    if isinstance(arg, ast.Call) and arg.func.id == 'locals':
AttributeError: 'Attribute' object has no attribute 'id'

BigRoy · 2023-09-01T15:35:48Z

Unfortunately I did end up using system_settings on our end for a Deadline creator plug-in.

See here to use it here

See here to use it here.

Correction: These are not actually creator plug-ins - but they are publishing plug-ins. Does this PR affect those too? Looking at the changed files it doesn't but I'm assuming a PR for that might be incoming too? So it'd be good to maybe figure out how to solve the issue I describe below for both cases.

Any good ideas on how I could still optimally retrieve our globally define deadline URL - since the project settings URL just picks one by name from the Module Settings? The only thing I can think of is that if in the apply_settings I'd initialize a ModuleManager I could access deadline_module.deadline_urls but I'd still need to do that per plug-in to get the instantiated modules at the time of apply_settings so I couldn't get it from a module instance either without this becoming slow.

Also, since this is a backwards incompatible change for external code bases (e.g. anyone having a custom new style creator using apply_settings) it might be good to merge this into a next minor release maybe with a clear note in the changelog?

iLLiCiTiT · 2023-09-01T15:52:19Z

Does this PR affect those too?

No.

Looking at the changed files it doesn't but I'm assuming a PR for that might be incoming too?

Publish plugins already have changed code to allow expect only project settings, but it is "less radical" there, because system settings are still used, which won't change until full AYON release.
I've chose more radical approach in create plugins, because I didn't find any usage of them anywhere and couldn't think of the usage...

Also, since this is a backwards incompatible change for external code bases (e.g. anyone having a custom new style creator using apply_settings) it might be good to merge this into a next minor release maybe with a clear note in the changelog?

I would rather change it to be more backwards compatible than wait for next-minor, last next-minor took more than 6 months.

EDITED: But I don't think it would break anything. System settings does not contain much relevant information for create plugins.

BigRoy · 2023-09-01T16:01:23Z

For Create plug-ins I'm personally fine with removing it - I'd just need to sync up potential custom creators but if it's clear WHEN I should be doing that then I'm totally fine with it. It's a trivial change. Plus it moves the cleanup to now instead of pushing it into future for forever.

hound

Some files could not be reviewed due to errors:

Traceback (most recent call last):

Traceback (most recent call last):
  File "/usr/local/bin/flake8", line 8, in 
    sys.exit(main())
  File "/usr/local/lib/python3.8/dist-packages/flake8/main/cli.py", line 18, in main
    app.run(argv)
  File "/usr/local/lib/python3.8/dist-packages/flake8/main/application.py", line 393, in run
    self._run(argv)
  File "/usr/local/lib/python3.8/dist-packages/flake8/main/application.py", line 381, in _run
    self.run_checks()
  File "/usr/local/lib/python3.8/dist-packages/flake8/main/application.py", line 300, in run_checks
    self.file_checker_manager.run()
  File "/usr/local/lib/python3.8/dist-packages/flake8/checker.py", line 331, in run
    self.run_serial()
  File "/usr/local/lib/python3.8/dist-packages/flake8/checker.py", line 315, in run_serial
    checker.run_checks()
  File "/usr/local/lib/python3.8/dist-packages/flake8/checker.py", line 598, in run_checks
    self.run_ast_checks()
  File "/usr/local/lib/python3.8/dist-packages/flake8/checker.py", line 502, in run_ast_checks
    for (line_number, offset, text, check) in runner:
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 56, in run
    parser.visit(self.tree)
  File "/usr/lib/python3.8/ast.py", line 371, in visit
    return visitor(node)
  File "/usr/lib/python3.8/ast.py", line 379, in generic_visit
    self.visit(item)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 39, in visit_ClassDef
    self.capture_issues_visitor('ClassDef', node)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 33, in capture_issues_visitor
    self.generic_visit(node)
  File "/usr/lib/python3.8/ast.py", line 381, in generic_visit
    self.visit(value)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 36, in visit_Call
    self.capture_issues_visitor('Call', node)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 30, in capture_issues_visitor
    issues = checker.run(node)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checkers/render.py", line 22, in run
    if isinstance(arg, ast.Call) and arg.func.id == 'locals':
AttributeError: 'Attribute' object has no attribute 'id'

iLLiCiTiT · 2023-09-01T16:18:06Z

Added backwards compatibility, System settings are still passed to Create plugin. A warning message is logged if __init__ is overriden or apply_settings still expect system_settings.

openpype/pipeline/create/creator_plugins.py

hound

Some files could not be reviewed due to errors:

Traceback (most recent call last):

Traceback (most recent call last):
  File "/usr/local/bin/flake8", line 8, in 
    sys.exit(main())
  File "/usr/local/lib/python3.8/dist-packages/flake8/main/cli.py", line 18, in main
    app.run(argv)
  File "/usr/local/lib/python3.8/dist-packages/flake8/main/application.py", line 393, in run
    self._run(argv)
  File "/usr/local/lib/python3.8/dist-packages/flake8/main/application.py", line 381, in _run
    self.run_checks()
  File "/usr/local/lib/python3.8/dist-packages/flake8/main/application.py", line 300, in run_checks
    self.file_checker_manager.run()
  File "/usr/local/lib/python3.8/dist-packages/flake8/checker.py", line 331, in run
    self.run_serial()
  File "/usr/local/lib/python3.8/dist-packages/flake8/checker.py", line 315, in run_serial
    checker.run_checks()
  File "/usr/local/lib/python3.8/dist-packages/flake8/checker.py", line 598, in run_checks
    self.run_ast_checks()
  File "/usr/local/lib/python3.8/dist-packages/flake8/checker.py", line 502, in run_ast_checks
    for (line_number, offset, text, check) in runner:
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 56, in run
    parser.visit(self.tree)
  File "/usr/lib/python3.8/ast.py", line 371, in visit
    return visitor(node)
  File "/usr/lib/python3.8/ast.py", line 379, in generic_visit
    self.visit(item)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 39, in visit_ClassDef
    self.capture_issues_visitor('ClassDef', node)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 33, in capture_issues_visitor
    self.generic_visit(node)
  File "/usr/lib/python3.8/ast.py", line 381, in generic_visit
    self.visit(value)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 36, in visit_Call
    self.capture_issues_visitor('Call', node)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 30, in capture_issues_visitor
    issues = checker.run(node)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checkers/render.py", line 22, in run
    if isinstance(arg, ast.Call) and arg.func.id == 'locals':
AttributeError: 'Attribute' object has no attribute 'id'

jakubjezek001

Nuke and TrayPublisher tested and worked normally

antirotor

Creators in Maya, Houdini, 3dsmax and Blender works

kalisp

Tested in PS, AE, works.

BigRoy · 2023-09-05T15:26:10Z

openpype/pipeline/create/creator_plugins.py

+        init_overriden = self.__class__.__init__ is not BaseCreator.__init__
+        if init_overriden or expect_system_settings:


Ok, so this warning is now always logged for all classes inheriting from the new Creator due to the Creator class already overriding __init__.

I guess we'll need to hotfix?

iLLiCiTiT added 3 commits September 1, 2023 15:54

only project settings are passed by default to create plugin

78b5ae1

fixed typehints

1f6df90

removed deprecated 'abstractproperty'

fee3c95

ynbot added type: enhancement Enhancements to existing functionality size/XS Denotes a PR changes 0-99 lines, ignoring general files labels Sep 1, 2023

iLLiCiTiT added 10 commits September 1, 2023 17:07

apply settings in TVPaint does not expect system settings

e2c6083

removed system settings usage from traypublisher

21d5d4c

fusion creator does not expect system settings

cb3ef02

removed duplicated attribute

09d31e8

removed usage of system settings from after effects

3ba7939

removed usage of system settings from houdini

539120f

maya create plugins do not expecte system settings

cb3e831

photoshop create plugins do not expect system settings

9338c16

add missing tvpaint creators

7441d26

nuke create plugins do not expect system settings

ce3e9a5

hound bot reviewed Sep 1, 2023

View reviewed changes

iLLiCiTiT requested review from BigRoy, kalisp, antirotor and jakubjezek001 September 1, 2023 15:22

iLLiCiTiT assigned kalisp Sep 1, 2023

ynbot added host: After Effects host: Fusion host: Houdini host: Maya host: Nuke host: Photoshop host: TrayPublisher host: TV Paint host: UE labels Sep 1, 2023

backwards compatibility is kept with waring message

88ab56d

hound bot reviewed Sep 1, 2023

View reviewed changes

BigRoy reviewed Sep 1, 2023

View reviewed changes

openpype/pipeline/create/creator_plugins.py Show resolved Hide resolved

openpype/pipeline/create/creator_plugins.py Outdated Show resolved Hide resolved

Fix versions in deprecated message

35074e8

hound bot reviewed Sep 4, 2023

View reviewed changes

jakubjezek001 approved these changes Sep 4, 2023

View reviewed changes

antirotor approved these changes Sep 5, 2023

View reviewed changes

kalisp approved these changes Sep 5, 2023

View reviewed changes

kalisp assigned iLLiCiTiT and unassigned kalisp Sep 5, 2023

iLLiCiTiT merged commit 2b2209c into develop Sep 5, 2023

iLLiCiTiT deleted the enhancement/remove-system-settings-requirement branch September 5, 2023 10:00

ynbot added this to the next-patch milestone Sep 5, 2023

BigRoy reviewed Sep 5, 2023

View reviewed changes

iLLiCiTiT mentioned this pull request Sep 5, 2023

Create Plugins: Better check of overriden '__init__' method #5571

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chore: Create plugin does not expect system settings #5553

Chore: Create plugin does not expect system settings #5553

iLLiCiTiT commented Sep 1, 2023 •

edited by kalisp

Loading

hound bot left a comment

BigRoy commented Sep 1, 2023 •

edited

Loading

iLLiCiTiT commented Sep 1, 2023 •

edited

Loading

BigRoy commented Sep 1, 2023 •

edited

Loading

hound bot left a comment

iLLiCiTiT commented Sep 1, 2023

hound bot left a comment

jakubjezek001 left a comment

antirotor left a comment

kalisp left a comment

BigRoy Sep 5, 2023 •

edited

Loading

		init_overriden = self.__class__.__init__ is not BaseCreator.__init__
		if init_overriden or expect_system_settings:

Chore: Create plugin does not expect system settings #5553

Chore: Create plugin does not expect system settings #5553

Conversation

iLLiCiTiT commented Sep 1, 2023 • edited by kalisp Loading

Changelog Description

Additional info

Testing notes:

hound bot left a comment

Choose a reason for hiding this comment

BigRoy commented Sep 1, 2023 • edited Loading

iLLiCiTiT commented Sep 1, 2023 • edited Loading

BigRoy commented Sep 1, 2023 • edited Loading

hound bot left a comment

Choose a reason for hiding this comment

iLLiCiTiT commented Sep 1, 2023

hound bot left a comment

Choose a reason for hiding this comment

jakubjezek001 left a comment

Choose a reason for hiding this comment

antirotor left a comment

Choose a reason for hiding this comment

kalisp left a comment

Choose a reason for hiding this comment

BigRoy Sep 5, 2023 • edited Loading

Choose a reason for hiding this comment

iLLiCiTiT commented Sep 1, 2023 •

edited by kalisp

Loading

BigRoy commented Sep 1, 2023 •

edited

Loading

iLLiCiTiT commented Sep 1, 2023 •

edited

Loading

BigRoy commented Sep 1, 2023 •

edited

Loading

BigRoy Sep 5, 2023 •

edited

Loading