Skip to content
This repository was archived by the owner on Nov 22, 2024. It is now read-only.

Add a flag to disable Flink in the operator#1017

Merged
andreaTP merged 4 commits into
masterfrom
disable-flink-operator
Apr 9, 2021
Merged

Add a flag to disable Flink in the operator#1017
andreaTP merged 4 commits into
masterfrom
disable-flink-operator

Conversation

@andreaTP

@andreaTP andreaTP commented Apr 8, 2021

Copy link
Copy Markdown
Contributor

No description provided.

@andreaTP andreaTP requested a review from RayRoestenburg April 8, 2021 14:24

private def podReady(ps: App.PodStatus) = {
ps.status == PodStatus.Running && ps.nrOfContainersReady == ps.nrOfContainers && ps.nrOfContainers > 0
(ps.status == PodStatus.Running && ps.nrOfContainersReady == ps.nrOfContainers && ps.nrOfContainers > 0) || (ps.nrOfContainers == 0 && ps.nrOfContainersReady == 0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem right..

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC the nrOfContainers can be 0 at first. If this is just for flink status when flink is disabled, rather use the "" marker string to decide this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

nrOfContainers: Int): App.PodStatus = {
val ready =
if (nrOfContainersReady == nrOfContainers && nrOfContainers > 0) PodStatus.ReadyTrue else PodStatus.ReadyFalse
if ((nrOfContainersReady == nrOfContainers && nrOfContainers > 0)) PodStatus.ReadyTrue else PodStatus.ReadyFalse

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

expectedPodCount = Some(0),
podStatuses = Seq(
App.PodStatus(
name = "<external>",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not depend on this "<external>" string to identify the podstatus that should be treated differently?

@RayRoestenburg RayRoestenburg self-requested a review April 8, 2021 14:52

@RayRoestenburg RayRoestenburg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to discuss the ready logic.

private def getStatusFromContainerStates(containerStates: List[ContainerState], nrOfContainers: Int): String =
if (containerStates.nonEmpty) {
private def getStatusFromContainerStates(containerStates: List[ContainerState], nrOfContainers: Int): String = {
if (nrOfContainers == 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would prefer to show as pending? not sure about this change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverted

@andreaTP

andreaTP commented Apr 8, 2021

Copy link
Copy Markdown
Contributor Author

should be good to go now @RayRoestenburg

@RayRoestenburg RayRoestenburg self-requested a review April 8, 2021 18:04
@andreaTP andreaTP merged commit a133396 into master Apr 9, 2021
@andreaTP andreaTP deleted the disable-flink-operator branch April 9, 2021 07:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants