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

Settings: Migrate settings based on bundle #326

Conversation

martastain
Copy link
Member

@martastain martastain commented Aug 21, 2024

PR Checklist

  • migrate settings by bundle function
  • migrateSettingsByBundle endpoint
  • extend save bundle endpoint to allow settings migration in one step
  • handle site settings properly
  • dispatch settings changed events
  • use in-method caching of settings to reduce query amount
  • log everything

Testing

Phase 1: Test migrateSettingsByBundle endpoint

  • Turn off developer mode
  • Create OldBundle bundle with an older version of an addon and change its studio and project settings
  • Create NewBundle bundle with a newer version of the same addon and keep it untouched
  • Set OldBundle to production and NewBundle to staging

Call (using postman, curl or httpie) a [POST] request with administrator or service privileges to /api/migrateSettingsByBundle with the following JSON payload

{
  "sourceBundle": "OldBundle",
  "targetBundle": "NewBundle",
  "sourceVariant": "production",
  "targetVariant": "staging"
}
  • Check the logs for errors. Failed to validate XXXX with value XXX warnings are expected if the target addon does not have migration method in place. that setting won't be migrated.

  • Ensure the settings were correctly propagated in studio settings and project settings

  • When Settings: copy project settings when promoting a bundle #311 is merged, use "promote bundle" function on the staging bundle and ensure site settings were also migrated

@BigRoy
Copy link
Contributor

BigRoy commented Aug 22, 2024

Check the logs for errors. Failed to validate XXXX with value XXX warnings are expected if the target addon does not have migration method in place. that setting won't be migrated.

Could you elaborate on this? Would it never work if an addon doesn't specify a certain function? Or it would just be unable to resolve conflicts using use a migration method if version mismatches would have mismatching settings.

And when saying "mismatching settings". Is that, incompatible settings, like a type changing? Or if e.g. a setting was removed or added would it also fail?

@martastain
Copy link
Member Author

Could you elaborate on this? Would it never work if an addon doesn't specify a certain function? Or it would just be unable to resolve conflicts using use a migration method if version mismatches would have mismatching settings.

And when saying "mismatching settings". Is that, incompatible settings, like a type changing? Or if e.g. a setting was removed or added would it also fail?

If the model changes fields in an incompatible way such as list[str] to str and the newer addon does not implement convert_settings_overrides method handling this change ( https://github.com/ynput/ayon-backend/blob/develop/ayon_server/addons/addon.py#L545 ) that field will be skipped during migration and the default will be used.

It should however handle simpler changes such as int -> str and we could expand this implicit conversions based on experience. It uses the very same method that is used in the "copy settings dialog" in the frontend.

@martastain martastain marked this pull request as ready for review August 28, 2024 09:40
@martastain martastain merged commit 0259572 into develop Aug 30, 2024
@martastain martastain deleted the 325-settings-optional-settings-migration-based-on-the-bundle branch October 21, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature Adding something new and exciting to the product
Projects
None yet
2 participants