Skip to content
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

Add config tab for Prometheus and multi-cluster support #67

Merged
merged 10 commits into from
Jan 15, 2025

Conversation

jrauh01
Copy link
Collaborator

@jrauh01 jrauh01 commented Aug 20, 2024

Add a new config tab for Prometheus. If the config is provided via yaml the data is locked and the form is disabled.

@jrauh01 jrauh01 self-assigned this Aug 20, 2024
@cla-bot cla-bot bot added the cla/signed CLA is signed by all contributors of a PR label Aug 20, 2024
@jrauh01 jrauh01 force-pushed the prometheus-config branch 2 times, most recently from f1f5bc1 to aa99d38 Compare August 22, 2024 12:26
@lippserd lippserd added this to the 0.3.0 milestone Aug 25, 2024
@jrauh01 jrauh01 changed the base branch from auto-create-notifications-source to main November 19, 2024 13:35
@jrauh01 jrauh01 force-pushed the prometheus-config branch 5 times, most recently from b7d2590 to b0b211e Compare November 20, 2024 15:45
@jrauh01 jrauh01 force-pushed the prometheus-config branch 5 times, most recently from 93f5449 to c8a4321 Compare December 18, 2024 16:12
@lippserd lippserd changed the title Add config tab for Prometheus Add config tab for Prometheus and multi-cluster support Jan 14, 2025
@@ -80,53 +70,69 @@ public function hasBeenSubmitted(): bool
};

$form = (new NotificationsConfigForm())
->setKConfig($kconfig)
->populate(['cluster_uuid' => $this->getRequest()->get('id')])
Copy link
Member

Choose a reason for hiding this comment

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

Please rename id to cluster_uuid and retrieve the default value from the session. This ensures that any changes a user makes to the displayed cluster are automatically reflected when working with the config. If the session is empty, we still populate null so that the form defaults to the first database entry in the cluster table.

}

Notification::success(
$this->translate('New configuration has successfully been stored.')
);

$this->redirectNow('__REFRESH__');
$this->redirectNow(Url::fromPath('kubernetes/config/notifications', ['id' => $clusterUuid]));
Copy link
Member

Choose a reason for hiding this comment

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

Rename id to cluster_uuid.

public function prometheusAction()
{
$form = (new PrometheusConfigForm())
->populate(['cluster_uuid' => $this->getRequest()->get('id')])
Copy link
Member

Choose a reason for hiding this comment

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

Please rename id to cluster_uuid and get the default value from the session.

}

Notification::success($this->translate('New configuration has successfully been stored'));
$this->redirectNow(Url::fromPath('kubernetes/config/prometheus', ['id' => $clusterUuid]));
Copy link
Member

Choose a reason for hiding this comment

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

Rename id to cluster_uuid and add a newline before this line.

@lippserd
Copy link
Member

And please also reword your wip commit.

@jrauh01 jrauh01 requested a review from lippserd January 14, 2025 13:12
@lippserd lippserd merged commit 5089bfb into main Jan 15, 2025
5 checks passed
@lippserd lippserd deleted the prometheus-config branch January 15, 2025 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla/signed CLA is signed by all contributors of a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants