#108 Allow certain configured groups to by-pass the PR-only setting
Merged 3 years ago by pingou. Opened 3 years ago by pingou.

file modified
+9 -1
@@ -67,6 +67,9 @@ 

              re.compile(refre) for refre in pagure_config["RCM_BRANCHES"]

          ]

          self.rcm_group = pagure_config.get("RCM_GROUP")

+         self.bypass_pr_only_groups = set(

+             pagure_config.get("BYPASS_PR_ONLY_GROUPS") or []

+         )

          self.supported_sigs = set(pagure_config.get("SUPPORTED_SIGS", []))

          self.sig_prefixes = pagure_config.get("SIG_PREFIXES", [])

          self.protected_namespaces = pagure_config.get(
@@ -189,6 +192,10 @@ 

          # Determine whether the user is RCM

          is_rcm = bool(self.rcm_group and self.rcm_group in usergroups)

  

+         # Determine if the user is part of a group which allows them to

+         # by-pass the PR-only setting

+         bypass_pr_only = self.bypass_pr_only_groups.intersection(usergroups)

+ 

          # If configured, print user info

          self.debug("Protected namespaces: %s" % self.protected_namespaces)

          self.debug("Blocking unspecified refs: %s" % self.block_unspecified)
@@ -198,6 +205,7 @@ 

          self.debug("Committer: %s" % is_committer)

          self.debug("SIG memberships: %s" % user_sigs)

          self.debug("RCM: %s" % is_rcm)

+         self.debug("By-pass PR-only: %s" % bool(bypass_pr_only))

  

          # Quick reminder about the protected_namespace: git.centos.org is

          # hosting in one pagure instance a "regular" git forge as well as
@@ -266,7 +274,7 @@ 

  

          # This is applicable to all namespace, protected or not

  

-         if repotype == "main" and not is_rcm:

+         if repotype == "main" and not is_rcm and not bypass_pr_only:

              # If this project has PRs only on, or PRs are globally enforced and

              # this is not a fork, only allow pushing if this is a PR merge.

              # However, RCM/releng is allowed to by-pass the PR only model

Pagure has a setting that can be set for each project to disallow
direct commit and instead have all contribution be made via
pull-requests.
On dist-git (src.fp.o), if enabled, this feature has the unforseen
consequences of blocking provenpackager or releng from fixing
packages directly and forcing them into this PR-only model.
This is not ideal as we want provenpackager or releng to be able to
fix quickly packages that need to be.

This issue was raised in https://pagure.io/fedora-infrastructure/issue/8895

Fixes https://pagure.io/fedora-infrastructure/issue/8895

Signed-off-by: Pierre-Yves Chibon pingou@pingoured.fr

It's obvious what you do here (and the code likely works), but the "by_pass_pr_only = ..." construct is still somewhat hard to read.

How about making self.bypass_pr_only_groups a set right away and using the intersection operator (the last one is syntactical sugar, feel free to ignore)? Also, consistently use bypass without underscore and just utilize that lists/tuple/sets/string are understood as False(-ish) if empty and True(-ish) if not. E.g.:

    self.bypass_pr_only_groups = set(pagure_config.get("BYPASS_PR_ONLY_GROUPS", []))
    ...
    # Determine if the user is part of a group which allows them to by-pass the PR-only setting
    bypass_pr_only = self.bypass_pr_only_groups & usergroups
    ...
    self.debug("Bypass PR-only: %s" % bool(bypass_pr_only))
    ...
    if repotype == "main" and not is_rcm and not bypass_pr_only:

I've had to tweak:

    self.bypass_pr_only_groups = set(pagure_config.get("BYPASS_PR_ONLY_GROUPS", []))

to

    self.bypass_pr_only_groups = set(pagure_config.get("BYPASS_PR_ONLY_GROUPS") or [])

as if the configuration file has: BYPASS_PR_ONLY_GROUPS = None the .get() will give you what is in the config, ie: None and poof :)

I did keep the .intersection() which I find clearer (though longer) in what it does.

The rest is adjusted, thanks for the advices!

rebased onto 8a5941f485eb849fc85b8a6e09c948f689e990c7

3 years ago

This needs to be bypass_pr_only (without underscore) and should be wrapped in bool(bypass_pr_only) so that the debug output contains a simple True or False rather than the intersection of the group sets.

This also has to be bypass_pr_only.

rebased onto 1117155

3 years ago

Pull-Request has been merged by pingou

3 years ago
Metadata