-
Notifications
You must be signed in to change notification settings - Fork 60
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
[DXEX-2581] Add support for Actions log sessions endpoint #127
Conversation
Codecov ReportBase: 95.38% // Head: 95.39% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #127 +/- ##
==========================================
+ Coverage 95.38% 95.39% +0.01%
==========================================
Files 37 37
Lines 6068 6085 +17
==========================================
+ Hits 5788 5805 +17
Misses 226 226
Partials 54 54
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
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 contribution folks! Just left a comment about the endpoint behavior to assess if we can remove the pointers from those fields, otherwise everything looks good 👍🏻
Thanks for the feedback @sergiught I've applied the suggestions and push the commit. |
URL *string `json:"url,omitempty"` | ||
Expires *time.Time `json:"expires,omitempty"` | ||
|
||
Filters []ActionLogSessionFilter `json:"filters,omitempty"` |
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.
I was able to find the json schema for this in api2 and filters has minItems: 1, so we can remove the omit empty here as well and from the other fields as well the pointers.
cc: @FadyMak
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.
@sergiught filters
is not a required field so we need to have omitempty
here otherwise it would result in a 400 if we pass in an empty filters
array.
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.
Ok that makes sense, but what about URL and Expires? They seem to be required properties in the response so we can remove the pointers.
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.
@sergiught these properties only exist on the response, not the request so they should have omitempty
otherwise the Management API will throw a 400.
🔧 Changes
Adds support for creating a log session via the Management API for tailing Actions Logs.
🔬 Testing
Tested against a space with the following steps:
Step 1: Generate Log Session
Step 2: Subscribe to Stream
Step 3: Execute an Action
Execute an Action from the Dashboard or via the Authentication API and observe the event data for the specific Action being filtered.
📝 Checklist