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 checks execution domain logic #256

Merged
merged 16 commits into from
Mar 31, 2022
Merged

Conversation

fabriziosestito
Copy link
Member

@fabriziosestito fabriziosestito commented Mar 29, 2022

This PR is the first iteration of the runner checks execution.
It adds the domain logic in the cluster aggregate and the commands and events needed to perform an execution process.
It also temporarily fixes the old FE so we can have some user feedback (review, yay).

ATM we are leveraging the MockRunner in the development environment and ignoring the host connection settings.
More to come!

@fabriziosestito fabriziosestito force-pushed the add_runner_callbacks_endpoint branch 2 times, most recently from 3d7fcbc to 59eb80e Compare March 29, 2022 15:05
@fabriziosestito fabriziosestito marked this pull request as ready for review March 29, 2022 15:08
@fabriziosestito fabriziosestito changed the title Add runner callbacks endpoint Add checks execution domain logic Mar 29, 2022
)

# TODO: post event to callback
Copy link
Member Author

Choose a reason for hiding this comment

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

remove TODO

Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

Amazing, amazing job @fabriziosestito
I have noted down some discussion points, but the change looks super promising.

PD: This looks a crutial part of the project, and I think a mob review would be ideal

assets/js/components/ChecksResults.jsx Show resolved Hide resolved
assets/js/components/ClustersList.jsx Show resolved Hide resolved
"http://localhost:4000/api/runner/callback",
Jason.encode!(%{
"event" => "execution_started",
"cluster_id" => cluster_id,
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to implement this in the runner part. I have already forgotten hehe

Copy link
Member

@nelsonkopliku nelsonkopliku left a comment

Choose a reason for hiding this comment

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

Crazy journey 😄

Great iteration!

Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

YES

Wow you did actually fry your brain for this. I'm impressed! 😆

@@ -17,9 +15,8 @@ defmodule Trento.CheckResultReadModel do
field :cluster_id, Ecto.UUID, primary_key: true
field :host_id, Ecto.UUID, primary_key: true
field :check_id, :string, primary_key: true
field :result, Ecto.Enum, values: [:passing, :warning, :critical, :running]
field :result, Ecto.Enum, values: [:passing, :warning, :critical, :unknown]
Copy link
Contributor

Choose a reason for hiding this comment

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

Other small comment. From the runner, we pass the skipped result as well to show which were the skipped checks.

Copy link
Member Author

Choose a reason for hiding this comment

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

noted, will come in follow up PRs

@fabriziosestito fabriziosestito force-pushed the add_runner_callbacks_endpoint branch from 59eb80e to 6beed13 Compare March 30, 2022 10:32
@fabriziosestito fabriziosestito force-pushed the add_runner_callbacks_endpoint branch 2 times, most recently from 5665e29 to f2ba0b1 Compare March 31, 2022 08:43
@fabriziosestito fabriziosestito merged commit 7e52180 into main Mar 31, 2022
@fabriziosestito fabriziosestito deleted the add_runner_callbacks_endpoint branch March 31, 2022 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants