-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: allow TTL to be a duration in days, weeks, etc.. #9345
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: iTrooz <[email protected]>
Signed-off-by: iTrooz <[email protected]>
Signed-off-by: iTrooz <[email protected]>
Signed-off-by: iTrooz <[email protected]>
|
Is this really needed? |
Signed-off-by: iTrooz <[email protected]>
Signed-off-by: iTrooz <[email protected]>
Signed-off-by: iTrooz <[email protected]>
Signed-off-by: iTrooz <[email protected]>
Signed-off-by: iTrooz <[email protected]>
I believe so, entering backup TTL in hours can be annoying/error prone. For example, I wanted to create a 30d backup TTL for my personal cluster, and "30d" is more readable than "720h" But this is clearly a quality-of-life addition, it doesn't allow new things |
Signed-off-by: iTrooz <[email protected]>
Co-authored-by: Tiger Kaovilai <[email protected]> Signed-off-by: iTrooz <[email protected]>
|
don't forget to regenerate crd :) |
Signed-off-by: iTrooz <[email protected]>
|
Whoops, thank you |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9345 +/- ##
==========================================
+ Coverage 59.67% 59.91% +0.23%
==========================================
Files 383 384 +1
Lines 44079 34991 -9088
==========================================
- Hits 26306 20966 -5340
+ Misses 16223 12479 -3744
+ Partials 1550 1546 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
kaovilai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some further doc or implementation changes I think are needed.
Signed-off-by: iTrooz <[email protected]>
Signed-off-by: iTrooz <[email protected]>
Signed-off-by: iTrooz <[email protected]>
Signed-off-by: iTrooz <[email protected]>
Signed-off-by: iTrooz <[email protected]>
Signed-off-by: iTrooz <[email protected]>
Signed-off-by: iTrooz <[email protected]>
|
Hello ! Could someone else please review this PR ? |
Thank you for contributing to Velero!
Please add a summary of your change
This PR changes the type used to parse TTLs, allowing parsing types largers than hours. The most obvious benefit is to allow days, but I also added weeks, months, and years
Does your change fix a particular issue?
No
Please indicate you've done the following:
make new-changelog) or comment/kind changelog-not-requiredon this PR.site/content/docs/main.Notes
For now, this feature has the following limitations:
I would be glad to add these changes, if there is an interest for it