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

fix(logging): Entries has a 24H default filter #3120

Merged
merged 7 commits into from Apr 7, 2021
Merged

fix(logging): Entries has a 24H default filter #3120

merged 7 commits into from Apr 7, 2021

Conversation

@0xSage
Copy link
Contributor

@0xSage 0xSage commented Oct 31, 2020

Would like to push this through as a minor feat, and a nonbreaking change. User's existing code will actually not be broken by the PR. Users will simply retrieve less logs upon running listEntries. Previously, users would have hit API quota anyway. This is more a bug fix than a new feature

fixes: #3088

BREAKING CHANGE: significantly speeds up call to list log entries
Copy link
Member

@codyoss codyoss left a comment

I have a couple of questions about these changes, raised internally.

@0xSage
Copy link
Contributor Author

@0xSage 0xSage commented Nov 2, 2020

After discussion: we'll batch this change with future improvements

logging/logadmin/logadmin.go Outdated Show resolved Hide resolved
logging/logadmin/logadmin.go Show resolved Hide resolved
@0xSage 0xSage requested a review from as a code owner Nov 16, 2020
@0xSage 0xSage changed the title feat(logging)!: Entries has a 24H default filter feat(logging): Entries has a 24H default filter Apr 6, 2021
@0xSage 0xSage changed the title feat(logging): Entries has a 24H default filter fix(logging): Entries has a 24H default filter Apr 6, 2021
@0xSage 0xSage requested a review from codyoss Apr 6, 2021
codyoss
codyoss approved these changes Apr 7, 2021
Copy link
Member

@codyoss codyoss left a comment

Seems reasonable and a more desirable behavior. It looks like this was also considered a non-breaking change in the node libraries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants