Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Refactor Integrate Asset #3530

Merged
merged 100 commits into from
Jul 22, 2022
Merged

Refactor Integrate Asset #3530

merged 100 commits into from
Jul 22, 2022

Conversation

mkolar
Copy link
Member

@mkolar mkolar commented Jul 18, 2022

This is a followup PR from #2898 which was merged to an intermediate branch and we've added a few commits.

To be able to merge this, we've moved hosts and families filters for both the old and the new integrator to their respective GUI settings. The names of classes don't help here, but we can't easily rename the original class as we need to keep production backwards compatibility with existing project settings overrides.

The settings are here
project_settings/global/publish/IntegrateAssetNew (the original integrator)
project_settings/global/publish/IntegrateAsset (the refactored integrator)

This PR makes all of the hosts and families default to the refactored integrator. The plan is to merge do develop and look out for any issues that appear while testing any other PRs or production deployments. As soon as an issue appears, that particular combination of host and family should be moved to the legacy integrator in the settings. That action will revert it to the original and won't affect production, however, it will also automatically start generating a list of problematic combinations that need to be addressed in further PRs on the path to eradicate the old and ugly integrator plugin.

Please make sure that before you deploy develop, or next minor release into production, someone has the rights to quickly change system settings on these two plugins in case of any production issues.


Instructions for fixing issues cause by this PR

If something breaks during integration, you can hotfix it by editing the exclusion filters in settings project_settings/global/publish/IntegrateAsset.

You can exclude any combination of host + family and at from that moment the legacy integrator will be used instead. This should allow you to fix any problems cause on the spot without even restarting your DCC. If you need to do this at any point, please make sure to report the host + family combination to us, so it can be looked at.

This change will make it into the next patch release, considering it can be reverted selectively and doesn't need a new build.

# Conflicts:
#	openpype/plugins/publish/integrate_new.py
… in CollectAnatomyContextData and CollectAnatomyInstanceData.

This currently was duplicated logic and should not be handled in the Integrator
- This is moved from the Integrate Asset New settings
@mkolar mkolar requested a review from BigRoy July 18, 2022 09:54
@iLLiCiTiT
Copy link
Member

iLLiCiTiT commented Jul 19, 2022

Modified code:

  • New integrator has same host and families filters as legacy integrator (all of them)
  • New integrator marks instances for legacy integrator to skip them and avoid double integration
  • Make sure that order of legacy integrator is after new integrator
  • New integrator has settings where is possible to "skip" host and families combinations to easily fix production when something breaks. If instance is skipped using this settings it will be integrated using legacy integrator.
  • Settings for publish templates are taken from legacy integrator
    • they will be moved elsewhere out of plugin settings in future PR (this is to avoid break compatibility of settings twice)
  • New integrator does not crash if all representations are marked for deletion, and representations filtering happens before registering
  • CollectSubsetGroup was renamed to IntegrateSubsetGroup and was changed it's order to happen in integration
  • few minor changes

Explanations (To avoid confusion)

  • New integrator: Plugin IntegrateAsset in ./openpype/plugins/publish/integrate.py
  • Legacy integrator: Plugin IntegrateAssetNew in ./openpype/plugins/publish/integrate_legacy.py

@mkolar mkolar assigned iLLiCiTiT and unassigned mkolar Jul 19, 2022
@kalisp
Copy link
Member

kalisp commented Jul 19, 2022

It worked on basic published in:

  • AE
  • PS
  • Nuke

@iLLiCiTiT iLLiCiTiT force-pushed the feature/refactor_integrator branch from b532a20 to 037ed71 Compare July 22, 2022 07:58
@mkolar
Copy link
Member Author

mkolar commented Jul 22, 2022

Just fixed the exclusion condition. Ready to merge

@mkolar mkolar merged commit 152066c into develop Jul 22, 2022
@mkolar mkolar deleted the feature/refactor_integrator branch July 22, 2022 13:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: refactor Structural changes not affecting functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants