-
Notifications
You must be signed in to change notification settings - Fork 129
Delete old versions #445
Delete old versions #445
Conversation
Resolves #396 |
This is crashing with our latest avalon core, but that is a bug in the core itself. @iLLiCiTiT could you please have a look? |
@mkolar the fix is here; ynput/avalon-core#176 |
Ah yeah. Sorry my bad |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's working pretty well for a first iteration of the action. there are a few caveats. Currently it only prints the calculated filesize if the debug mode is on, because it only prints to console. Secondly after deleting the library loader crashes when clicked :). @iLLiCiTiT will be able to advise about that.
I think the easiest would be to trigger But I'm not sure if that will trigger refresh of subsets widget. If not then must be executed |
Cool, I can have a look at that later. @iLLiCiTiT any thoughts on implementing Loader settings into the Library Loader? |
I thought they are there? Created PR ynput/avalon-core#192 |
Loader options in library are now merged to 2.x/develop |
Thanks. Added an item to check on the description. |
46534d7
to
461dc0e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some files could not be reviewed due to errors:
./pype/tools/pyblish_pype/model.py:965:34: B008: Do not perform function call...
./pype/tools/pyblish_pype/model.py:965:34: B008: Do not perform function calls in argument defaults. The call is performed only once at function definition time. All calls to your function will reuse the result of that definition-time function call. If this is intended, assign the function call to a module-level variable and use that variable as a default value. ./pype/tools/pyblish_pype/model.py:969:31: B008: Do not perform function calls in argument defaults. The call is performed only once at function definition time. All calls to your function will reuse the result of that definition-time function call. If this is intended, assign the function call to a module-level variable and use that variable as a default value. ./pype/tools/pyblish_pype/model.py:994:41: B008: Do not perform function calls in argument defaults. The call is performed only once at function definition time. All calls to your function will reuse the result of that definition-time function call. If this is intended, assign the function call to a module-level variable and use that variable as a default value.
Traceback (most recent call last):
Traceback (most recent call last): File "/home/linters/.local/bin/flake8", line 11, in sys.exit(main()) File "/home/linters/.local/lib/python3.6/site-packages/flake8/main/cli.py", line 18, in main app.run(argv) File "/home/linters/.local/lib/python3.6/site-packages/flake8/main/application.py", line 393, in run self._run(argv) File "/home/linters/.local/lib/python3.6/site-packages/flake8/main/application.py", line 381, in _run self.run_checks() File "/home/linters/.local/lib/python3.6/site-packages/flake8/main/application.py", line 300, in run_checks self.file_checker_manager.run() File "/home/linters/.local/lib/python3.6/site-packages/flake8/checker.py", line 331, in run self.run_serial() File "/home/linters/.local/lib/python3.6/site-packages/flake8/checker.py", line 315, in run_serial checker.run_checks() File "/home/linters/.local/lib/python3.6/site-packages/flake8/checker.py", line 598, in run_checks self.run_ast_checks() File "/home/linters/.local/lib/python3.6/site-packages/flake8/checker.py", line 502, in run_ast_checks for (line_number, offset, text, check) in runner: File "/home/linters/.local/lib/python3.6/site-packages/flake8_django/checker.py", line 56, in run parser.visit(self.tree) File "/usr/lib/python3.6/ast.py", line 253, in visit return visitor(node) File "/usr/lib/python3.6/ast.py", line 261, in generic_visit self.visit(item) File "/home/linters/.local/lib/python3.6/site-packages/flake8_django/checker.py", line 39, in visit_ClassDef self.capture_issues_visitor('ClassDef', node) File "/home/linters/.local/lib/python3.6/site-packages/flake8_django/checker.py", line 33, in capture_issues_visitor self.generic_visit(node) File "/usr/lib/python3.6/ast.py", line 263, in generic_visit self.visit(value) File "/home/linters/.local/lib/python3.6/site-packages/flake8_django/checker.py", line 36, in visit_Call self.capture_issues_visitor('Call', node) File "/home/linters/.local/lib/python3.6/site-packages/flake8_django/checker.py", line 30, in capture_issues_visitor issues = checker.run(node) File "/home/linters/.local/lib/python3.6/site-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'
e77bdb0
to
b09b419
Compare
- Replaced settings dialog with Loader options. - Replaced print statements with logging and message boxes.
There is currently an issue when you run this on multiple assets, where the size is calculated per asset rather than a total of all the assets. This is because the Loader action is run per asset rather than on all the assets. Guessing this is a design implementation of the Loader we'll have to live with? |
I'm afraid so, at least for now. But I'm trying it out and it's looking good. |
I was thinking whether there was a Loader class or modify the existing Loader class , so the Loader gets passed a list of assets instead of a single one? |
Actually since the context is being passed to the Loader, the context just needs another key |
|
||
options = [ | ||
qargparse.Integer( | ||
"versions", default=2, min=1, help="Versions to keep:" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should allow minimum to 0, which effectively deletes the full version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
however in that case, the subset should be deleted with it.
qargparse.Integer( | ||
"versions", default=2, min=1, help="Versions to keep:" | ||
), | ||
qargparse.Boolean("publish", help="Remove publish folder:") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To keep this clear to the user I'd change the attribute names to nicer versions to keep
and delete folder
. I know the behaviour and caught myself mistaking whether it's version to keep or delete multiple time during testing.
this kind of ties to the fact, that you see it on multiple representations, but it's really a version level action, so it's confusing. Maybe it could be fixed at the same time. @iLLiCiTiT what do you think? |
Happy to merge to an intermediate branch, for some final touches on our end. |
Up to you guys. I'm happy to keep this PR open until we have something that is more production ready, ei. not having pops up per asset. |
…d_versions Delete old versions
Unfortunately the nature of Loaders is that they operate on one representation at a time, so when multiple subsets are selected the settings dialog opens multiple times.
This could be solved with Loader options.
I was also thinking of splitting out the calculate to a different Loader, so better separate destructive actions.