-
Notifications
You must be signed in to change notification settings - Fork 7
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
Implemented event steam #8
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Roman Bylbas <romka.bulbas@gmail.com>
Signed-off-by: Roman Bylbas <romka.bulbas@gmail.com>
|
||
private ?string $name = null; | ||
|
||
private ?array $data = null; |
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.
@RomanovSci we need to add typing here. I guess, the most convenient way would be to have event-specific event classes. The parsing could be moved to the EventStream
.
Note, that events include the standard Casper types inside of them.
docs/Example/EventStream.md
Outdated
} | ||
); | ||
|
||
$es->connect(\Casper\EventStream\EventSource::EVENT_TYPE_MAIN); |
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.
The API is a bit confusing here:
- We create an
EventSource
object and connect it to event source, which isn't logical thus isn't intuitive - We check if event id is not
null
, why? If that's part of the logic it should be handled by the library
Separatelly, we should provide possibility to start from the predefined event id, which could be passed in the event stream URL in the start_from
parameter.
Note, the following possible usage:
$nodeUrl = 'http://localhost:9999';
$streamPath = \Casper\EventStream\EventSource::EVENT_TYPE_MAIN;
$startFromEvent = 12345;
$stream = new Casper\EventStream($nodeUrl, $streamPath, $startFromEvent);
$es->onEvent(
function (\Casper\EventStream\DeployProcessedEvent $event) {
echo json_encode($event->getDeployHeader()->getHash()) . "\n";
}
);
$stream->listen();
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.
Regarding 2.
We just show how we can use abort()
method
src/EventStream/EventSource.php
Outdated
$this->url = $url; | ||
} | ||
|
||
public function onMessage(callable $callback): void |
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.
onEvent
would be a better name
src/EventStream/EventSource.php
Outdated
($this->onMessage)($event); | ||
} | ||
} catch (\Exception $_) { | ||
} |
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.
Why emtpy catch?
src/EventStream/EventSource.php
Outdated
/** | ||
* @throws \Exception | ||
*/ | ||
public function connect(string $eventsType, int $startFrom = 0): void |
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.
$startFrom
isn't used anywhere- A better name would be
listen
This function offers a blocking event processing. I guess it would be nice to offer an async interface. This way a consumer could subscribe to several event streams simultaneously and process all the events at once if needed. With the current approach it's not possible to process let's say main event stream and the deploys one.
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.
@RomanovSci please check my comments
Signed-off-by: Roman Bylbas <romka.bulbas@gmail.com>
@RomanovSci It looks like we have a small conflict on the README file. Could you please take a look? |
# Conflicts: # README.md
Signed-off-by: Roman Bylbas <romka.bulbas@gmail.com>
Signed-off-by: Roman Bylbas <romka.bulbas@gmail.com>
Signed-off-by: Roman Bylbas romka.bulbas@gmail.com
Summary
Implemented event steam
Checklist