-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Deprecated default_report_months #27350
Conversation
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.
Just a couple of things that I think should be addressed before merging.
CI Results: |
Build Results: |
1641a74
to
560107e
Compare
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.
Super close, @divyaac! Just a couple more items and then we should be good to merge.
Co-authored-by: Mike Palmiotto <mike.palmiotto@hashicorp.com>
Co-authored-by: Sarah Chavis <62406755+schavis@users.noreply.github.com>
Co-authored-by: Mike Palmiotto <mike.palmiotto@hashicorp.com>
Co-authored-by: Mike Palmiotto <mike.palmiotto@hashicorp.com>
df20a55
to
f931826
Compare
endpoint, will result in the following warning from Vault: | ||
<CodeBlockConfig hideClipboard> | ||
```shell-session | ||
default_report_months is deprecated: defaulting to billing start time |
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.
It might be more helpful to show an actual request/output, as a user might see it.
Not really a blocker, so if it's a pain I can approve once the other items are addressed.
Co-authored-by: Mike Palmiotto <mike.palmiotto@hashicorp.com>
Co-authored-by: Mike Palmiotto <mike.palmiotto@hashicorp.com>
Co-authored-by: Mike Palmiotto <mike.palmiotto@hashicorp.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.
Good stuff, @divyaac! Looks like this is ready to merge.
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.
Addressed the changes with the resolved commits!
Matching ENT PR: https://github.com/hashicorp/vault-enterprise/pull/5929
closes https://github.com/hashicorp/vault-enterprise/pull/5929