-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
fix(onboarding): deprecate misleading retentionPeriodHrs
key
#20798
Changes from 4 commits
a35b134
2066d59
2528d90
4b8e2b1
7b1dffa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,20 +30,25 @@ func onboardingRequest(ui *input.UI, options *options) (*influxdb.OnboardingRequ | |
} | ||
|
||
req := &influxdb.OnboardingRequest{ | ||
User: options.target.userName, | ||
Password: options.target.password, | ||
Org: options.target.orgName, | ||
Bucket: options.target.bucket, | ||
RetentionPeriod: influxdb.InfiniteRetention, | ||
Token: options.target.token, | ||
User: options.target.userName, | ||
Password: options.target.password, | ||
Org: options.target.orgName, | ||
Bucket: options.target.bucket, | ||
RetentionPeriodSeconds: influxdb.InfiniteRetention, | ||
Token: options.target.token, | ||
} | ||
|
||
if options.target.retention != "" { | ||
dur, err := internal.RawDurationToTimeDuration(options.target.retention) | ||
if err != nil { | ||
return nil, err | ||
} | ||
req.RetentionPeriod = dur | ||
secs := dur / time.Second | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above, can you use the Duration.Seconds() call? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds like a good idea, I'll play with the math |
||
nanos := dur % time.Second | ||
if nanos > 0 { | ||
return nil, fmt.Errorf("retention policy %q is too precise, must be divisible by 1s", options.target.retention) | ||
} | ||
req.RetentionPeriodSeconds = int64(secs) | ||
} | ||
|
||
if options.force { | ||
|
@@ -72,16 +77,16 @@ func onboardingRequest(ui *input.UI, options *options) (*influxdb.OnboardingRequ | |
"Please type your retention period in hours.\nOr press ENTER for infinite", infiniteStr) | ||
rp, err := strconv.Atoi(rpStr) | ||
if rp >= 0 && err == nil { | ||
req.RetentionPeriod = time.Duration(rp) * time.Hour | ||
req.RetentionPeriodSeconds = int64((time.Duration(rp) * time.Hour).Seconds()) | ||
break | ||
} | ||
} | ||
} | ||
|
||
if confirmed := internal.GetConfirm(ui, func() string { | ||
rp := "infinite" | ||
if req.RetentionPeriod > 0 { | ||
rp = fmt.Sprintf("%d hrs", req.RetentionPeriod/time.Hour) | ||
if req.RetentionPeriodSeconds > 0 { | ||
rp = (time.Duration(req.RetentionPeriodSeconds) * time.Second).String() | ||
} | ||
return fmt.Sprintf(` | ||
You have entered: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,12 +28,13 @@ type OnboardingResults struct { | |
// OnboardingRequest is the request | ||
// to setup defaults. | ||
type OnboardingRequest struct { | ||
User string `json:"username"` | ||
Password string `json:"password"` | ||
Org string `json:"org"` | ||
Bucket string `json:"bucket"` | ||
RetentionPeriod time.Duration `json:"retentionPeriodHrs,omitempty"` | ||
Token string `json:"token,omitempty"` | ||
User string `json:"username"` | ||
Password string `json:"password"` | ||
Org string `json:"org"` | ||
Bucket string `json:"bucket"` | ||
RetentionPeriodSeconds int64 `json:"retentionPeriodSeconds,omitempty"` | ||
RetentionPeriodHours time.Duration `json:"retentionPeriodHrs,omitempty"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Self-review: I meant to name this |
||
Token string `json:"token,omitempty"` | ||
} | ||
|
||
func (r *OnboardingRequest) Valid() error { | ||
|
@@ -59,3 +60,10 @@ func (r *OnboardingRequest) Valid() error { | |
} | ||
return nil | ||
} | ||
|
||
func (r *OnboardingRequest) RetentionPeriod() time.Duration { | ||
if r.RetentionPeriodSeconds > 0 { | ||
return time.Duration(r.RetentionPeriodSeconds) * time.Second | ||
} | ||
return r.RetentionPeriodHours | ||
} |
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.
Would it be better here to use the Duration.Seconds() and Duration.Nanoseconds() to hide the internal representation in the Duration? The modulo will have to change, if you do this, of course.