-
Notifications
You must be signed in to change notification settings - Fork 2.1k
[Feature][Zeta] Report non-terminal job states #10133
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
base: dev
Are you sure you want to change the base?
Conversation
...-engine-server/src/test/java/org/apache/seatunnel/engine/server/event/JobStateEventTest.java
Show resolved
Hide resolved
...-engine-server/src/test/java/org/apache/seatunnel/engine/server/event/JobStateEventTest.java
Show resolved
Hide resolved
...ngine-server/src/main/java/org/apache/seatunnel/engine/server/dag/physical/PhysicalPlan.java
Show resolved
Hide resolved
|
Hi @zhangshenghang , PTAL when you have time. Thanks! |
chl-wxp
left a comment
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.
docs/zh/concept/event-listener.md, please modify this document as well.
| url: "http://example.com:1024/event/report" | ||
| headers: | ||
| Content-Type: application/json | ||
| report-non-terminal-job-state: false |
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.
Add parameter introduction, usage, and precautions.
| log.info( | ||
| String.format( | ||
| "%s turned from state %s to %s.", jobFullName, current, targetState)); | ||
| if (!targetState.isEndState() |
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 INITIALIZING and CREATED statuses will also be reported. Is this reasonable?
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.
Actually, the INITIALIZING and CREATED states are not reported by this method.
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.
Actually, the
INITIALIZINGandCREATEDstates are not reported by this method.
Why? Did I read it wrong? But in the org.apache.seatunnel.engine.common.job.JobStatus enumeration, both INITIALIZING and CREATED are marked as EndState.NOT_END.
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 INITIALIZING and CREATED states are not reported here, because the updateJobState method is not used to update those two states.
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
INITIALIZINGandCREATEDstates are not reported here, because theupdateJobStatemethod is not used to update those two states.
Got it, thank you for your patient answer.
Refer to: #9842
Purpose of this pull request
This PR implements the feature for reporting non-terminal job states.
Does this PR introduce any user-facing change?
Yes.
Users can receive non-terminal job state events by enabling the
report-non-terminal-job-stateoption.How was this patch tested?
Added a test in
JobStateEventTest.Check list
New License Guide
incompatible-changes.mdto describe the incompatibility caused by this PR.