-
Notifications
You must be signed in to change notification settings - Fork 14
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
Complementing support to /v2/events #12
Conversation
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 PR! Could you please also add some documentation/examples, because I find this feature really interesting.
@@ -62,6 +63,21 @@ function Marathon(url, opts) { | |||
} | |||
} | |||
|
|||
if (requestStream) { | |||
return new Promise((resolve, reject) => { |
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.
Please rewrite to normal function, since this lib is not using latest JavaScript standard. We will introduce Babel or whatever in future.
Sure @pnedelko ! When we attach to the Marathon Event Bus (from the Nodejs perspective) we receive a Readable Stream object. Here is an example on the user side on how to implement it using the
Any further explanation about this feature or documentation that I missed, please let me know! |
@castanhob could you please add this code example to readme file? |
Done. Could you please check if it fits the README properly? |
Well done! Thank you! |
First of all, thanks for this amazing lib and work!
This pull request adds a required header for the
/v2/events
endpoint, which needs theAccept: text/event-stream
to connect (following the Marathon API Documentation).Not only that, but since
request-promise
currently has problems resolving promises with streams (check request/request-promise#90)and even their documentation states that we must use the
request
lib to resolve those ones, I made a small change to be able to resolve this promise and return the stream to be managed by the user following his needs.