sidetag-koji-plugin

#1 Multiple extensions
Closed by ignatenkobrain. Opened by tkopecek.
tkopecek/sidetag-koji-plugin extensions  into  master

Download 1.patch

I've done some changes, so opening a PR to discuss them. Maybe it would be worth to split it to more PRs - let's see if it will not be too confusing.

  • Tag name is rewritten to use real tag_id instead of get_event - it means wasting one number in tag_id sequence
  • user's count of sidetags is propagated to policy handler
  • sidetag removal (mikem)
  • automatic sidetag removal after last build is untagged (mikem) - this one triggers a possible bug in koji. It must be fixed to use this, there is no simple workaround.
  • CLI (add-sidetag, remove-sidetag, list-sidetags)

ping @mikem, @mizdebsk
Edited by tkopecek

7 new commits added

  • list sidetags
  • CLI plugin
  • add policy data
  • missing imports
  • callback to remove sidetags when they become empty
  • avoid direct queries and provide remove call
  • partial

1 new commit added

  • configurable auto-delete

1 new commit added

  • fix default config name

1 new commit added

  • fix typos

2 new commits added

  • simple message for policy violation
  • fix typo

I think that either strict=True needs to be passed, or return value of multiCall() checked.

Why are these marked as admin commands? Ordinary non-admin users are expected to be able to create sidetags by themselves.

I think that arches should not be defined, but default value of None should be used. This way sidetags will inherit arches from parent tag. Changes to arches of base tag will be propagated to side tags, without having to modify all them manually.

It would be nice to have queryOpts argument and pass it to QueryProcessor

A custom count query should be faster than readTaggedBuilds, especially when there are lots of builds tagged.

Thanks @tkopecek, the PR looks good in general. I didn't test it yet, but I will do that today.

4 new commits added

  • refactor basetag names + docs
  • faster sql query
  • add strict to tag removal
  • remove admin prefix in CLI

1 new commit added

  • copy arches from basetag

Considering this is now running in production, should we get it merged?

argparse? optparse is dead for long long time.

/usr

1 new commit added

  • fix typo

1 new commit added

  • replace optarse with argparse

19 new commits added

  • replace optarse with argparse
  • fix typo
  • copy arches from basetag
  • refactor basetag names + docs
  • faster sql query
  • add strict to tag removal
  • remove admin prefix in CLI
  • simple message for policy violation
  • fix typo
  • fix typos
  • fix default config name
  • configurable auto-delete
  • list sidetags
  • CLI plugin
  • add policy data
  • missing imports
  • callback to remove sidetags when they become empty
  • avoid direct queries and provide remove call
  • partial

I believe we can drop this entirely since plugin should not work with 1.17.x since it does not have _create_build_target and such functions. So I would drop this comment.

I believe we can drop this entirely since plugin should not work with 1.17.x since it does not have _create_build_target and such functions. So I would drop this comment.

Hmm, referenced PR is not merged yet. So, even 1.19 is broken for this functionality. I've planned it for 1.20 now.

1 new commit added

  • update comment

So I have merged it as part of other PR, let's close this. Now all changes from here are in the master. Just 7 month later, not even 9 ;)

Pull-Request has been closed by ignatenkobrain

I'd have rather liked to see this one go in and your PR be on the top of it allowing for clearer distinction between the original intent and yours.
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.