Skip to content

Conversation

@njucjc
Copy link
Contributor

@njucjc njucjc commented Apr 30, 2025

Thank you for contributing to Harbor!

Comprehensive Summary of your change

Add rate limiter for Alibaba Cloud ACR adapter

Please indicate you've done the following:

  • Well Written Title and Summary of the PR
  • Label the PR as needed. "release-note/ignore-for-release, release-note/new-feature, release-note/update, release-note/enhancement, release-note/community, release-note/breaking-change, release-note/docs, release-note/infra, release-note/deprecation"
  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Made sure tests are passing and test coverage is added if needed.
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.

@njucjc njucjc requested a review from a team as a code owner April 30, 2025 09:22
@njucjc njucjc force-pushed the add_rate_limiter branch from 34a2a32 to 7a6c37e Compare April 30, 2025 09:24
@codecov
Copy link

codecov bot commented Apr 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.49%. Comparing base (c8c11b4) to head (30e7dd7).
Report is 482 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #21953      +/-   ##
==========================================
+ Coverage   45.36%   46.49%   +1.12%     
==========================================
  Files         244      253       +9     
  Lines       13333    14238     +905     
  Branches     2719     2925     +206     
==========================================
+ Hits         6049     6620     +571     
- Misses       6983     7263     +280     
- Partials      301      355      +54     
Flag Coverage Δ
unittests 46.49% <ø> (+1.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 178 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@chlins chlins added release-note/update Update or Fix replication/adapters related to replication adapters labels May 6, 2025
@wy65701436 wy65701436 self-assigned this May 6, 2025
@njucjc njucjc requested a review from chlins May 9, 2025 09:03
@chlins
Copy link
Member

chlins commented May 13, 2025

I suggest the rate_limit_transport.go can be moved to src/lib.

@njucjc njucjc force-pushed the add_rate_limiter branch 4 times, most recently from 0a76d2d to bcee0d1 Compare May 13, 2025 09:10
@njucjc njucjc force-pushed the add_rate_limiter branch from bcee0d1 to 47d3534 Compare May 15, 2025 09:08
Copy link
Member

@chlins chlins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@njucjc njucjc force-pushed the add_rate_limiter branch 2 times, most recently from bf35cdb to 3899f1b Compare May 15, 2025 11:43
@njucjc
Copy link
Contributor Author

njucjc commented May 15, 2025

@wy65701436 PTAL

Copy link
Contributor

@wy65701436 wy65701436 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@njucjc njucjc force-pushed the add_rate_limiter branch from 3899f1b to 59ad9b3 Compare May 27, 2025 06:59
var envTcrQPSLimit, _ = strconv.Atoi(os.Getenv("TCR_QPS_LIMIT"))
if envTcrQPSLimit > 1 && envTcrQPSLimit < tcrQPSLimit {
tcrQPSLimit = envTcrQPSLimit
var envTcrQPSLimit, _ = strconv.Atoi(os.Getenv("REG_ADAPTER_TCR_QPS_LIMIT"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kofj Please take a look, because as this refactor, the env key of rate limit for TCR has been changed.

@chlins chlins force-pushed the add_rate_limiter branch from 59ad9b3 to 30e7dd7 Compare May 27, 2025 07:41
@chlins chlins enabled auto-merge (squash) May 27, 2025 07:41
@chlins chlins merged commit a546f99 into goharbor:main May 27, 2025
12 checks passed
OrlinVasilev pushed a commit to OrlinVasilev/harbor that referenced this pull request Oct 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-note/update Update or Fix replication/adapters related to replication adapters target/2.14.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants