-
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: Migrated bucket should have correct retention policy. #17769
Conversation
Signed-off-by: Patrik Helia <patashelia@gmail.com>
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.
Thanks for the PR! Looks great. One minor stylistic change requested. Also, can you run go fmt
so the CI linter passes?
$ go fmt tsdb/migrate/data_v1.go
tsdb/migrate/migrate.go
Outdated
f, err := os.Open(file) | ||
if err != nil { | ||
if os.IsNotExist(err) { | ||
return nil, err | ||
} | ||
return nil, err | ||
} | ||
defer f.Close() | ||
|
||
data, err := ioutil.ReadAll(f) | ||
if err != nil { | ||
return nil, err | ||
} |
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.
This can all be shortened to:
data, err := ioutil.ReadFile(file)
if err != nil {
return nil, err
}
Also, your check for os.IsNotExist(err)
on L308 is redundant since both error cases simply return err
.
Signed-off-by: Patrik Helia <patrik.helia@kiwi.com>
@patriczek Thanks for the update to your commit! lgtm |
@benbjohnson You are welcome |
This commit fixed that, if is created bucket while migration and the old database had set retention policy, than this policy is used in created bucket.
Closes #17257
Signed-off-by: Patrik Helia patashelia@gmail.com