-
Notifications
You must be signed in to change notification settings - Fork 13
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
API filter on events and results #179
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #179 +/- ##
=========================================
- Coverage 74.52% 74.32% -0.2%
=========================================
Files 57 58 +1
Lines 1162 1192 +30
=========================================
+ Hits 866 886 +20
- Misses 226 237 +11
+ Partials 70 69 -1
Continue to review 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.
I was wondering what happen when a client kill the connection on the api ListenEvent
for example.
Is the associated goroutine automatically destroyed? If not, we could end up with too many subscriber on pubsub.Subscribe
.
Otherwise the code and logic seems ok. I didn't test yet.
I think this pull request should be done, the part to filter the data is not critical for now, we can do this in another pull request and also a bit tricky because we have to manage all the different types so let's keep this one simple and focus on more important developments. For the subscribe you are right now we don't unsubscribe every time a stream is disconnected this should be added |
…r-message-from-services # Conflicts: # CHANGELOG.md # cmd/service/test.go
|
…ask-filter, output-filter to match with the API.
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 renamed the flag of the service test
command to match with the name of the API.
I also improved the errors of validateEventKey
and validateTaskKey
.
Test.Flags().StringP("data", "d", "", "Path to the file containing the data required to run the task") | ||
Test.Flags().StringP("task-filter", "r", "", "Only log the result of the given task") | ||
Test.Flags().StringP("output-filter", "o", "", "Only log the data of the given output of a task result. If set, you also need to set the task in --result") | ||
Test.Flags().StringP("serviceID", "s", "", "ID of a previously deployed service") |
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.
Flags event
, result
, output
and service
doesn't match with the API.
I renamed them with the API names.
I create this #195 issue to track the improved version of the filter |
This allow to have more granularity on the events we listen from the core.
Also this request update the test command to be able to test an already deployed service and also keep a service alive
fix #23