HelpersTask676_Add_script_to_synchronize_GH_branch_protection_and_merge_property
Addressing #676
Documentation about YAML params - https://docs.google.com/document/d/1Kpe-DVpBjcy7Do4-NmnxlJkRcwjSMqwT5GFpjSE2qvo/edit?usp=sharing
I have included a way to revert GitHub settings to their defaults if they are present in the repo but not listed in the YAML manifest using the switch_default flag.
If a field is not present in the YAML file, NotSet is used to ensure no accidental overwrites.
@heanhsok, could you please review this code?
@heanhsok I tested the script with a test repository, and it works. I was unable to test the users and teams, but the backup file and switching to the default are working correctly.
Could you help check this? @sandeepthalapanane
user_1042@18f837e9d157:/app$ dev_scripts_helpers/github/dockerized_sync_gh_repo_settings.py --token_env_var GITHUB_PAT --input_file dev_scripts_helpers/github/settings/repo_settings.yaml --dry_run --owner HeaSoLab --repo demo-repository --backup
18:19:37 - INFO hdbg.py init_logger:1013 Saving log to file '/app/dev_scripts_helpers/github/dockerized_sync_gh_repo_settings.py.log'
18:19:37 - INFO hdbg.py init_logger:1018 > cmd='dev_scripts_helpers/github/dockerized_sync_gh_repo_settings.py --token_env_var GITHUB_PAT --input_file dev_scripts_helpers/github/settings/repo_settings.yaml --dry_run --owner HeaSoLab --repo demo-repository --backup'
18:19:38 - INFO dockerized_sync_gh_repo_settings.py _main:527 Settings backed up to /app/tmp.settings.HeaSoLab.demo-repository.yaml
Are you sure you want to synchronize repository settings? [y/n] y
18:19:40 - INFO dockerized_sync_gh_repo_settings.py apply_branch_protection:321 Would apply branch protection rules to main:
strict: True
contexts: ['ci/lint', 'ci/fasttests']
enforce_admins: True
dismiss_stale_reviews: True
require_code_owner_reviews: True
required_approving_review_count: 1
user_push_restrictions: ['admin1', 'admin2']
team_push_restrictions: ['maintainers', 'developers']
18:19:40 - INFO dockerized_sync_gh_repo_settings.py apply_repo_settings:415 Would apply repository settings:
name: test-repo
description: Test repository with branch protection and settings
homepage: https://github.com/test-repo
private: True
has_issues: True
has_projects: True
has_wiki: False
allow_squash_merge: True
allow_merge_commit: False
allow_rebase_merge: True
archived: False
Traceback (most recent call last):
File "/app/dev_scripts_helpers/github/dockerized_sync_gh_repo_settings.py", line 556, in <module>
_main(_parse())
File "/app/dev_scripts_helpers/github/dockerized_sync_gh_repo_settings.py", line 551, in _main
settings.apply_repo_settings(repo, args.dry_run)
File "/app/dev_scripts_helpers/github/dockerized_sync_gh_repo_settings.py", line 419, in apply_repo_settings
if enable_security_fixes is not None:
^^^^^^^^^^^^^^^^^^^^^
UnboundLocalError: cannot access local variable 'enable_security_fixes' where it is not associated with a value
Could you help check this? @sandeepthalapanane
user_1042@18f837e9d157:/app$ dev_scripts_helpers/github/dockerized_sync_gh_repo_settings.py --token_env_var GITHUB_PAT --input_file dev_scripts_helpers/github/settings/repo_settings.yaml --dry_run --owner HeaSoLab --repo demo-repository --backup 18:19:37 - INFO hdbg.py init_logger:1013 Saving log to file '/app/dev_scripts_helpers/github/dockerized_sync_gh_repo_settings.py.log' 18:19:37 - INFO hdbg.py init_logger:1018 > cmd='dev_scripts_helpers/github/dockerized_sync_gh_repo_settings.py --token_env_var GITHUB_PAT --input_file dev_scripts_helpers/github/settings/repo_settings.yaml --dry_run --owner HeaSoLab --repo demo-repository --backup' 18:19:38 - INFO dockerized_sync_gh_repo_settings.py _main:527 Settings backed up to /app/tmp.settings.HeaSoLab.demo-repository.yaml Are you sure you want to synchronize repository settings? [y/n] y 18:19:40 - INFO dockerized_sync_gh_repo_settings.py apply_branch_protection:321 Would apply branch protection rules to main: strict: True contexts: ['ci/lint', 'ci/fasttests'] enforce_admins: True dismiss_stale_reviews: True require_code_owner_reviews: True required_approving_review_count: 1 user_push_restrictions: ['admin1', 'admin2'] team_push_restrictions: ['maintainers', 'developers'] 18:19:40 - INFO dockerized_sync_gh_repo_settings.py apply_repo_settings:415 Would apply repository settings: name: test-repo description: Test repository with branch protection and settings homepage: https://github.com/test-repo private: True has_issues: True has_projects: True has_wiki: False allow_squash_merge: True allow_merge_commit: False allow_rebase_merge: True archived: False Traceback (most recent call last): File "/app/dev_scripts_helpers/github/dockerized_sync_gh_repo_settings.py", line 556, in <module> _main(_parse()) File "/app/dev_scripts_helpers/github/dockerized_sync_gh_repo_settings.py", line 551, in _main settings.apply_repo_settings(repo, args.dry_run) File "/app/dev_scripts_helpers/github/dockerized_sync_gh_repo_settings.py", line 419, in apply_repo_settings if enable_security_fixes is not None: ^^^^^^^^^^^^^^^^^^^^^ UnboundLocalError: cannot access local variable 'enable_security_fixes' where it is not associated with a value
I fixed this. Initially, I ran the script without a dry-run flag. I made sure that after the fix, I dry-ran the script and verified the implementation.
Sorry guys I've reviewed it while you guys were also working on it and tried my best to do a clean merge.
I've proposed several simplifications of the flow. The architecture and the use modes are not hidden.
Even if this is not production code, let's iterate to clarify the patterns as an exercise on understanding how to build elegant architecture.
Pls @sandeepthalapanane make a proposal with @heanhsok. @Shaunak01 keep an eye on this since this is a useful exercise for everyone.
@heanhsok
I have completely restructured the code following the review by @gpsaggese.
The new architecture flow includes:
-
Two distinct sub-commands: export and sync.
-
Export Pipeline: get settings → validate →save.
-
Sync Pipeline: load → validate → diff → log (dry-run) / backup → apply.
-
The central logic has been encapsulated in a private class called
_RepoAndBranchSettings, which builds repository and branch configurations once and reuses them. -
Backups are now created unconditionally in the current working directory, ensuring that restore points are always available.
-
The
switch_defaulthas been removed. It seems to be confusing and likely unnecessary, and we already have a common settings file, which looks enough.
Improvements made:
-
There is now comprehensive logging for dry-run operations, allowing users to see exactly what would change. Additionally, various logs are available for debug mode to better understand the flow of the code.
-
I have introduced key validation for YAML files (_validate_settings) to catch any unknown setting parameters.
-
Helper functions (_build_settings, _log_settings) have been factored out to eliminate duplicate code.@heanhsok I have completely restructured the code after @gpsaggese's review.
-
I have also added
currentandtargetprefixes to variable names for clarity. I made sure to use variable names likerepo_and_branch_settings,repo_settings, andbranch_settingsto avoid confusion. For example,repo_and_branch_settingscontains both repository and branch protection setting dictionaries, andrepo_settingscontains only repository settings. -
I have added more detailed comments, doc strings, and examples.