feat(flink): Describe command for Flink Jobs #758
Conversation
janelletavares
requested review from
nassor,
ericcheatham,
samirketema and
jayjayjpg
| _ builder.CommandWithSubCommands = (*Job)(nil) | ||
| _ builder.CommandWithFeatureFlag = (*Job)(nil) | ||
| _ builder.CommandWithHidden = (*Job)(nil) | ||
| ) | ||
|
|
||
| func (*Job) Usage() string { | ||
| return "flink" | ||
| return "jobs" |
There was a problem hiding this comment.
Thank you for the quick rename here as well!
| }, | ||
| { | ||
| {Align: simpletable.AlignRight, Text: "Input Streams:"}, | ||
| {Text: strings.Join(flinkJob.InputStreams, ", ")}, |
There was a problem hiding this comment.
Is this how you can parse a list into a comma-separated string?
There was a problem hiding this comment.
yes, this is the standard way to combine a slice of strings. Ilike spew.Sdump formatting for complicated structs, but this slice is simple
| }) | ||
| } | ||
|
|
||
| if flinkJob.Status.ReconciliationState != "" { |
There was a problem hiding this comment.
These guard checks are helpful for filtering out empty values, I'll keep this in mind for future CLI work as well
| @@ -85,8 +85,8 @@ func verifyPrintFlinkJobsOutput(t *testing.T, out string, flinkJob *meroxa.Flink | |||
| if !strings.Contains(out, flinkJob.UUID) { | |||
| t.Errorf("%s, not found", flinkJob.UUID) | |||
| } | |||
| if !strings.Contains(out, string(flinkJob.Status.State)) { | |||
| t.Errorf("state %s, not found", flinkJob.Status.State) | |||
| if !strings.Contains(out, string(flinkJob.Status.LifecycleState)) { | |||
There was a problem hiding this comment.
Do I understand it correctly, that for this phase of the work LifecycleState is the best representation of a flink job's state? Is it, because State will not be populated for now since the state computation will be implemented in a later stage of the project?
There was a problem hiding this comment.
That's our best guess for the time being, and it may change soon. I am looking forward to having one calculated state value, but we just don't know enough to do that yet.
Description of change
Fixes https://github.com/meroxa/turbine-project/issues/289
Type of change
How was this tested?
Demo
Additional references
Documentation updated