-
Notifications
You must be signed in to change notification settings - Fork 21
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
Use SSE to track CRC status change and logs #192
base: main
Are you sure you want to change the base?
Conversation
}, 3000); | ||
const crcVersion = await getCrcVersion(); | ||
if (!crcVersion) { | ||
console.warn("Can't get CRC CLI version, ignore getting daemons logs..."); |
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.
console.warn("Can't get CRC CLI version, ignore getting daemons logs..."); | |
console.warn('Can't get CRC CLI version, ignore getting daemons logs...'); |
LOL, I just re-requested, while you had just reviewed. Sorry. Just wanted to make sure you had seen this. |
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.
src/events/eventsource.ts
Outdated
let discardTrailingNewline = false; | ||
|
||
req.on('data', chunk => { | ||
if (!buf) { |
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.
a lot of processing to handle the end (linefeed) of a message. Can this be extracted and tested separately? At the moment, this looks more like implied knowledge.
src/events/eventsource.ts
Outdated
|
||
if (lineLength < 0) { | ||
startingPos = length - pos; | ||
startingFieldLength = fieldLength; |
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.
in this case it is a field
, right?
can we therefore parse this as:
this.parseEventStreamField(buf, pos, fieldLength)
?
And the other case as:
this.parseEventStreamLine(buf, pos, lineLength);
?
If seems now to rely on a singe function with another check. This looks more complex than it needs to be.
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.
That is for case when we don't receive 'full' line, as it is not guaranty that we will have whole line
so we just make sure that we have a complete line before parse it.
@jeffmaury Have you use CRC from crc-org/crc#3903 PR? The reason why you may have error with latest CRC is because of this PR was check for wrong version - #192 (comment) |
Signed-off-by: Yevhen Vydolob <[email protected]>
This PR depends on crc-org/crc#3903
PR adds usage new SSE channels for tracking CRC status change and receiving logs.