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

feat(warehouse): allow discovery interval and limit configurations #2038

Merged
merged 9 commits into from
May 21, 2024

Conversation

hiddeco
Copy link
Contributor

@hiddeco hiddeco commented May 17, 2024

This pull request adds the following things (as documented in the follow-up section of #1984):

  1. The option to define a custom interval at which the Warehouse attempts to discover new artifacts (instead of the 5m0s default).

    spec:
      interval: 1h0m
  2. The option (for each subscription) to limit artifacts (instead of the default of 20), with an upper limit of 100.

    spec:
      subscriptions:
      - git:
          discoveryLimit: 5
      - chart:
          discoveryLimit: 100
      - image:
          discoveryLimit: 25

hiddeco added 6 commits May 18, 2024 00:26
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Copy link

netlify bot commented May 17, 2024

Deploy Preview for docs-kargo-akuity-io ready!

Name Link
🔨 Latest commit 7548a95
🔍 Latest deploy log https://app.netlify.com/sites/docs-kargo-akuity-io/deploys/664c4a551ea50c000857187f
😎 Deploy Preview https://deploy-preview-2038.kargo.akuity.io
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Comment on lines +62 to +64
// +kubebuilder:validation:Type=string
// +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(s|m|h))+$"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a common workaround for the issue described here: kubernetes/apiextensions-apiserver#56

I took the further liberty to drop ms from the regex, as we probably do not want people to go below the seconds (and the level of precision in combination with e.g. minutes has little value).

@hiddeco hiddeco force-pushed the warehouse-discovery-interval branch from 5539107 to d07a867 Compare May 17, 2024 22:40
Copy link

codecov bot commented May 17, 2024

Codecov Report

Attention: Patch coverage is 88.23529% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 45.63%. Comparing base (881c012) to head (7548a95).
Report is 2 commits behind head on main.

Files Patch % Lines
internal/controller/warehouses/images.go 0.00% 1 Missing ⚠️
internal/controller/warehouses/warehouses.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2038   +/-   ##
=======================================
  Coverage   45.62%   45.63%           
=======================================
  Files         234      234           
  Lines       15998    16002    +4     
=======================================
+ Hits         7299     7302    +3     
- Misses       8342     8344    +2     
+ Partials      357      356    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hiddeco hiddeco force-pushed the warehouse-discovery-interval branch from d07a867 to fdecc2f Compare May 17, 2024 22:44
ID: meta.CommitID,
Tag: meta.Tag,
// A decent subject length for a commit message is 50 characters
// (based on the 50/72 rule). We are nice people, and allow a
Copy link
Member

Choose a reason for hiding this comment

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

We are nice people

😂 ❤️

Copy link
Member

@krancour krancour left a comment

Choose a reason for hiding this comment

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

Nice and clean.

@hiddeco
Copy link
Contributor Author

hiddeco commented May 20, 2024

@krancour when I woke up on Saturday I actually had a small improvement in mind, might be worth thinking about this so I can potentially implement it tomorrow.

Besides the discoveryLimit on subscription items, it could be an idea to also have an object global fallback default:

discovery:
  limit: 30
subscriptions:
- discoveryLimit: 100 # overwrite

Which could then also possibly result in:

discovery:
  interval: 1h0s # now nested

hiddeco added 3 commits May 21, 2024 09:11
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
@hiddeco hiddeco force-pushed the warehouse-discovery-interval branch from fdecc2f to 7548a95 Compare May 21, 2024 07:16
@hiddeco hiddeco added this pull request to the merge queue May 21, 2024
Merged via the queue into akuity:main with commit 81d11d4 May 21, 2024
17 checks passed
@hiddeco hiddeco deleted the warehouse-discovery-interval branch May 21, 2024 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants