-
Notifications
You must be signed in to change notification settings - Fork 119
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(pebble)!: change select=all to users=all for pebble get_notices #1146
fix(pebble)!: change select=all to users=all for pebble get_notices #1146
Conversation
ed33f52
to
01ea0de
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.
Excellent, thanks for this @IronCore864! Just a minor doc change, and also let's add a line to CHANGES.md for the next release.
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.
Looks great! No notes :)
Thanks @IronCore864 -- nice first PR! |
Description
Pebble added support for notices, aggregated events that are recorded with a type and key. The
/v1/notices
API had a parameter "select=all" to show notices for all users, and the parameter name is changed to "users=all" in recent release v1.9.0.This PR is the corresponding changes to the ops framework.
Related doc: OP042 - User ID features for Pebble Notices
Issue
See here: #1132
Changes
pebble.Client.get_notices
model.Container.get_notices
Unit Test
For pebble:
Some Manual Test
Start pebble:
Run some CLI commands like
pebble notify example.com/hello
.Import local code and test: