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

Add new throttler pkg forked from nozzle/throttler #108

Merged
merged 26 commits into from
Jul 23, 2024

Conversation

puerco
Copy link
Member

@puerco puerco commented Jul 23, 2024

What type of PR is this?

/kind feature

What this PR does / why we need it:

This commit imports a fork of the nozzle/throttler package.

As discussed in #107, this package is used in the release engineering tooling but has not had new commits in over six years so the release engineering team chose to adopt it to keep maintaining it.

Which issue(s) this PR fixes:

Follow-up to #107

Special notes for your reviewer:

I've imported the full commit history and added notices crediting the original package source and origin.

I also fixed the linter and updated the tests a little bit but they still need some improvement.

/cc @saschagrunert @xmudrii

Does this PR introduce a user-facing change?

- release-utils now has a new throttle package forked from nozzle/throttle

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 23, 2024
Copy link

linux-foundation-easycla bot commented Jul 23, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@k8s-ci-robot k8s-ci-robot added sig/release Categorizes an issue or PR as relevant to SIG Release. approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 23, 2024
puerco and others added 7 commits July 22, 2024 21:32
Co-authored-by: Nozzle <admin@nozzle.io>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
…or channel + errorCount

Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
puerco and others added 14 commits July 22, 2024 21:37
Co-authored-by: Joe Bergevin <josephbergevin@gmail.com>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Co-authored-by: Joe Bergevin <josephbergevin@gmail.com>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Co-authored-by: Joe Bergevin <josephbergevin@gmail.com>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
also changed order of channel sends/receives to send after updating atomic counters

Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
This commit imports the code from the nozzle/throttle module and imports it
into the release-utils repo as the relegn team will take care of maintaining
it.

The original module commit history is preserved.

Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
This commit fixes the copyright notices for the the throttler files
and adds a notice about the package fork.

Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
This commit mutes the linter warning for random/v2, for now we still
support go 1.21. Drop this commit and move to random/v2 when we bump
to go 1.22+

Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jul 23, 2024
I messed up during the import and applied the patches in
throttle/throttle. This commit moves the new package source to
the proper location. Sorry :(

Signed-off-by: Adolfo García Veytia (Puerco) <adolfo.garcia@uservers.net>
@puerco puerco changed the title Add new throtlle pkg forked from nozzle/throttle Add new throtlle pkg forked from nozzle/throttler Jul 23, 2024
@puerco puerco changed the title Add new throtlle pkg forked from nozzle/throttler Add new throttler pkg forked from nozzle/throttler Jul 23, 2024
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 23, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: puerco, saschagrunert

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [puerco,saschagrunert]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 55766d0 into kubernetes-sigs:main Jul 23, 2024
4 checks passed
@derekperkins
Copy link
Contributor

Not a big deal that you forked it, just surprising that you didn't file an issue first. There hasn't been a compelling reason for us to make any changes, so it wasn't unmaintained or abandoned, just considered complete, barring issue reports.

@puerco
Copy link
Member Author

puerco commented Jul 24, 2024

Hey @derekperkins thanks for chiming in!

I'm sorry, in the rush to get another feature in we didn't think this through. We should have filed an issue especially since we've been using the module for years, I'm happy to revert and we can contribute any patches back to you in the future. Apologies!

@justaugustus
Copy link
Member

Agreed on the revert here!

@puerco
Copy link
Member Author

puerco commented Jul 25, 2024

OK, opened #111 to revert the fork!

@derekperkins
Copy link
Contributor

No worries, really not a big deal either way. If anything, it was an ego booster that it was important enough to fork :)

I saw a few tweaked commits in this PR too that we're happy to pull in upstream, along with any other feedback you have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/release Categorizes an issue or PR as relevant to SIG Release. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants