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

Don't return a 204 if there's no historical data #17935

Merged
merged 2 commits into from
Nov 15, 2022

Conversation

raskchanky
Copy link
Contributor

Previously, if a Vault instance only had client count data for the current month, and you queried the activity log endpoint using dates that spanned historical data, you'd just get a 204 empty response. Now, instead, you get back the client counts for the current month. A concrete example might make things clearer:

Today is November 14. My Vault server came online for the first time on November 2. I've been using it ever since. If I query the activity log endpoint for 11/1/22 - 11/14/22, all is well. I get back partial results for this month. But, if I query for 10/1/22 - 11/14/22, previously I would get a 204 empty response. That's not correct though. I do have client count information for the time range requested, it's just all in November. With this patch, if I query for 10/1/22 - 11/14/22, I'll get back the same results that I would've gotten back had I queried from 11/1 - the partial results for November.

Copy link
Contributor

@ccapurso ccapurso left a comment

Choose a reason for hiding this comment

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

Looks good!

Comment on lines +1549 to +1551
// If the storedQuery is nil, that means there's no historical data to process. But, it's possible there's
// still current month data to process, so rather than returning a 204, let's proceed along like we're
// just querying the current month.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the comment documenting this scenario!

@raskchanky raskchanky merged commit 87aa644 into main Nov 15, 2022
@raskchanky raskchanky deleted the fix-activity-log-yet-again branch November 15, 2022 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants