Review: app config sync properties #2827
Open
+39
−65
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrismclarke
changed the base branch from
master
to
feat/app-config-sync-properties
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Checklist
Description
Targets branch of #2822
Refactor server sync frequency to deployment-level API config
Dev Notes
The main change here is to move the server sync configuration to deployment-level config instead of runtime, as it makes more sense for this to be a fixed value instead of template-dependent. As mentioned in the target issue, if wanting to block syncing for whatever reason I think we should probably build on top of this with a public method and/or authoring action that can disable sync.
I've also refactored the
db-sync.servicea little to share sync schedule with theserver.service. I was initially confused why there were two services performing similar tasks, but then remembered the original server service was designed for syncing to theapp_userstable and the db-sync any other table (currentlyfeedbackandapp_notification_interaction). That being said I think it makes sense to merge these together over time (or provider cleaner separation) as it feels a bit confusing, so for now I've moved all sync scheduling tasks to the server serviceGit Issues
Closes #
Screenshots/Videos
If useful, provide screenshot or capture to highlight main changes