feat(flink): Describe command for Flink Jobs#758
Conversation
janelletavares
requested review from
ericcheatham,
jayjayjpg,
nassor and
samirketema
jayjayjpg
left a comment
jayjayjpg
left a comment
There was a problem hiding this comment.
This reads great! I only left a few more questions to understand the feature better, but otherwise this is already good to go -- thank you for the super quick iteration! ✨
|
|
||
| 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
| } | ||
| 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.
3 participants
Description of change
Fixes https://github.com/meroxa/turbine-project/issues/289
Type of change
How was this tested?
Demo
Additional references
Documentation updated