Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shotgrid: reviving archived assets is broken #122

Merged

Conversation

kalisp
Copy link
Member

@kalisp kalisp commented Aug 13, 2024

Changelog Description

This PR supports recreation of children Tasks in AYON if deleted Asset is revived in Shotgrid.

Additional info

I don't like this solution too much, open for comments.

Main issue is that revival of assets on SG sends these events (in order) - attribute_change for CHILD task, attribute_change for Asset, revival_entity.
If we are processing first attribute_change, it wants to update non existing Task under non existing Asset.
Right now this throws new MissingParentError exception, that could be handled specifically.

It fails processing event, it creates new source event on shotgrid.event (there is no way how to create new event on processing topic, imho), which gets put on the end of the queue.
Then asset gets recreated, then this new event gets reprocessed (only once), this time hopefully it finds parent asset in AYON, so it finishes successfully (or fails forever).

Testing notes:

  1. create asset and task(s) in AYON
  2. let them be synchronized to SG (easiest to run in this repo service_tools/manage.ps services`)
  3. delete asset in AYON Editor
  4. let it delete asset in SG
  5. go to trash (top right your profile, Trash at the bottom, Asset)
    image
  6. check if asset and tasks under it got recreated in AYON

kalisp added 3 commits August 13, 2024 13:47
When asset with tasks is revived in SG, first attribute_change of child task is sent, then attribute_change for parent asset and then only entity_revival.
attribute_change of task is changed to create task.
attribute_change of asset is skipped, only entity_revival should be processed.
This could be raised if parent (eg. asset) is not yet recreated in AYON. It recreates event, puts it at the end of the queue(failing is not enough) and tries it once more, hopefully after Asset gets already created.
Added filtering only on regular events (starting with 'Leeched'), ignores `Recreated` events
@ynbot
Copy link
Contributor

ynbot commented Aug 13, 2024

Copy link

@MilaKudr MilaKudr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR works for me.

… enhancement/AY-5570_shotgrid-reviving-archived-assets-is-broken
@@ -244,8 +246,47 @@ def start_processing(self):
self,
payload,
)

except MissingParentError:
Copy link
Member

@iLLiCiTiT iLLiCiTiT Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we can't recreate parents instead of retriggering the event? Processor should not trigger events, it will add the event to the end of queue which might lead to huge issues as the task might change meanwhile, but you'll reapply changes from older event.

If this https://github.com/ynput/ayon-shotgrid/pull/122/files#r1718231580 happens because of this, then that is another reason against.

BTW is possible missing parent can happen even if it was not "reactivated"? Can it happen if the parents couldn't be synced (e.g. invalid chars of some other issue)?

@iLLiCiTiT
Copy link
Member

I am really against re-submitting leeched events in processor, it should not touch leeched events at all. At the first time of processing the event there might be other events changing or removing the same entity, so moving the event to the end of queue is a huge risk. There might be a real reason why parent was not synced, and re-submitting of the event can cause infinite loop of the event. It should be handled at the moment when we find out the parents are missing.

There also might be other services listening to leeched events, and re-submitting the events can cause their failure.

I'm not responsible for this addon, but for me this is no way.

Removed weird filtering on description, replaced with subtopic which is currently picked up too.
Creates simpler event.
kalisp and others added 2 commits August 19, 2024 10:54
Co-authored-by: Jakub Trllo <43494761+iLLiCiTiT@users.noreply.github.com>
Co-authored-by: Jakub Trllo <43494761+iLLiCiTiT@users.noreply.github.com>
@iLLiCiTiT
Copy link
Member

@MilaKudr could you re-test this please?

Copy link
Contributor

@BigRoy BigRoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cosmetics

services/shotgrid_common/ayon_shotgrid_hub/__init__.py Outdated Show resolved Hide resolved
services/processor/processor/processor.py Outdated Show resolved Hide resolved
kalisp and others added 4 commits September 10, 2024 15:04
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
@MilaKudr
Copy link

It is not working for me at the moment, after deleting the task in Ayon I get error messages like:
"2024-09-10 17:18:18.531 | update_from_ayon | remove_sg_entity_from_ayon_event | WARNING: Entity 'a82667b96f8711ef8ccdccd9acc131df' does not have a path to remove from Shotgrid.

… enhancement/AY-5570_shotgrid-reviving-archived-assets-is-broken
…iving-archived-assets-is-broken' into enhancement/AY-5570_shotgrid-reviving-archived-assets-is-broken
@kalisp
Copy link
Member Author

kalisp commented Sep 27, 2024

It is not working for me at the moment, after deleting the task in Ayon I get error messages like: "2024-09-10 17:18:18.531 | update_from_ayon | remove_sg_entity_from_ayon_event | WARNING: Entity 'a82667b96f8711ef8ccdccd9acc131df' does not have a path to remove from Shotgrid.

That was sorted out.

Currently its waiting for budget as there is a pushback for current implementation.

It shouldn't modify/create new messages, but re-create full folder structure if necessary. That is not trivial because of current code structure.

@kalisp kalisp marked this pull request as draft September 27, 2024 09:27
@BigRoy BigRoy requested review from BigRoy and removed request for BigRoy October 1, 2024 21:43
Now it tries to add parents in AYON instead of creating new event at the end of the queue.
@kalisp kalisp marked this pull request as ready for review October 14, 2024 15:34
@kalisp
Copy link
Member Author

kalisp commented Oct 14, 2024

I tried to rework logic without creating new event just by creating missing parents in the loop. Not most elegant code there though :/.

@tatiana-ynput tatiana-ynput added the sponsored This is directly sponsored by a client or community member label Oct 15, 2024
services/shotgrid_common/utils.py Outdated Show resolved Hide resolved
services/shotgrid_common/utils.py Show resolved Hide resolved
services/shotgrid_common/ayon_shotgrid_hub/__init__.py Outdated Show resolved Hide resolved
services/shotgrid_common/ayon_shotgrid_hub/__init__.py Outdated Show resolved Hide resolved
@jakubjezek001 jakubjezek001 added the type: bug Something isn't working label Oct 24, 2024
kalisp and others added 9 commits November 12, 2024 13:00
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
… enhancement/AY-5570_shotgrid-reviving-archived-assets-is-broken
…iving-archived-assets-is-broken' into enhancement/AY-5570_shotgrid-reviving-archived-assets-is-broken
retirement_date might come when retiring in SG, but will have filled new_value
Removal of tasks should be working.
@kalisp kalisp requested a review from BigRoy November 12, 2024 15:14
Copy link

@MilaKudr MilaKudr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works for assets and tasks

@kalisp kalisp merged commit 8e3b0db into develop Nov 13, 2024
1 check passed
@kalisp kalisp deleted the enhancement/AY-5570_shotgrid-reviving-archived-assets-is-broken branch November 13, 2024 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sponsored This is directly sponsored by a client or community member type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants