-
Notifications
You must be signed in to change notification settings - Fork 96
[JENKINS-38381] API to receive asynchronous notifications #29
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
Conversation
…f process output or exit code.
…ller call ProcessLiveness. It is guaranteed to not find the process, which would cause an immediate -1 return value. Instead limit the watcher to the simple result file, and check for special statuses only if exitStatus is being called from the master.
| } | ||
| CountingInputStream cis = new CountingInputStream(utf8EncodedStream); | ||
| handler.output(cis); | ||
| lastLocationFile.write(Long.toString(lastLocation + cis.getByteCount()), 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.
Seems that this would not suffer from JENKINS-37575-like problems. We intentionally write lastLocationFile only after a successful call to output, so that if there is an agent channel outage we do not lose output. In such a case this Watcher simply dies; if and when the agent is reconnected, a new one should start at the same position.
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.
Now applies to #60.
| try (FileChannel ch = FileChannel.open(Paths.get(logFile.getRemote()), StandardOpenOption.READ)) { | ||
| InputStream locallyEncodedStream = Channels.newInputStream(ch.position(lastLocation)); | ||
| InputStream utf8EncodedStream; | ||
| Charset nativeEncoding = Charset.defaultCharset(); |
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 assume this is taking the agent's default character set. If so ...
For various reasons (e.g. JENKINS-13091) we might be in a situation that the agent does not run with the systems default encoding, i.e. the –Dfile.encoding=… jvm parameter changing it to something else. Therefore system default encoding might be a good default. However, it should be possible to set the encoding explicitly since encoding of any shell output (IBM1047 on z/OS) does not necessarily match the encoding set by –Dfile.encoding.
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.
Resolved in #61.
|
Now identical to #62, which better captures the relationship among changes. |
Related to jenkinsci/workflow-job-plugin#27.
@reviewbybees esp. @oleg-nenashev