Receive facts gathered event #6
Conversation
fabriziosestito
force-pushed
the
receive_facts_gathered_event
branch
2 times, most recently
from
521004b
to
a424169
Compare
fabriziosestito
force-pushed
the
receive_facts_gathered_event
branch
from
c031d60
to
3605b4e
Compare
fabriziosestito
requested review from
nelsonkopliku,
arbulu89,
CDimonaco,
dottorblaster,
EMaksy and
rtorrero
There was a problem hiding this comment.
Super nice @fabriziosestito !
Nothing really to complain about hehe
|
|
||
| @facts_gathered_event "trento.checks.v1.FactsGathered" | ||
|
|
||
| @spec handle_event(CloudEvent.t()) :: :ok | {:error, any} |
There was a problem hiding this comment.
Is this signature correct?
Shouldn't return {:ok, [Fact.t()]} or similar?
There was a problem hiding this comment.
nope it returns :ok because it sends a message to the genserver
| defmodule Wanda.JsonSchema do | ||
| @moduledoc """ | ||
| TODO: move to separate repo. | ||
| """ | ||
|
|
||
| @schema_path Path.join(File.cwd!(), "priv/json_schema/") | ||
|
|
||
| for schema_file <- File.ls!(@schema_path) do | ||
| @external_resource schema_file | ||
|
|
||
| schema = | ||
| @schema_path | ||
| |> Path.join(schema_file) | ||
| |> File.read!() | ||
| |> Jason.decode!() | ||
| |> ExJsonSchema.Schema.resolve() | ||
|
|
||
| def validate(unquote(Path.basename(schema_file, ".schema.json")), data), | ||
| do: ExJsonSchema.Validator.validate(unquote(Macro.escape(schema)), data) | ||
| end | ||
|
|
||
| def validate(_, _), do: {:error, :schema_not_found} | ||
| end |
There was a problem hiding this comment.
I'm gonna print this and make some posters.
There was a problem hiding this comment.
or in the contract repo :P
|
|
||
| alias Wanda.Execution.Fact | ||
|
|
||
| @callback receive_facts(execution_id :: String.t(), agent_id :: String.t(), [Fact.t()]) :: |
There was a problem hiding this comment.
We are sometimes using the agent concept and sometimes the target concept.
Are we fine with the discrepancy, or should we aim to converge on naming?
Things are:
- we keep
agentfor the sake of simplicity and explicitness - we user
targetas a more abstract thing at wanda level
As a reminder, if that helps with the decision, we decided to go for group instead of cluster in wanda, in order to be open to the possibility that targets on which we do facts gathering for a check execution can be non necessarily part of the same cluster, but possibly also from different clusters or even different sap systems.
Do I remember correctly?
There was a problem hiding this comment.
@nelsonkopliku those are for sure related to me, but the agent_id is the id of an agent while the target is the struct representing a target execution (which also has agent_id inside, and the check list) so I'm not sure they need to be squashed to the same name.
I think Wanda can be agnostic about clusters but for sure agent is part of its concern.
There was a problem hiding this comment.
I see, I just thought that converging toward Target+target_id or Agent+agent_id would have been more consistent.
However, I'm completely fine with how it is.
|
|
||
| @schema_path Path.join(File.cwd!(), "priv/json_schema/") | ||
|
|
||
| for schema_file <- File.ls!(@schema_path) do |
| |> Jason.decode!() | ||
| |> ExJsonSchema.Schema.resolve() | ||
|
|
||
| def validate(unquote(Path.basename(schema_file, ".schema.json")), data), |
There was a problem hiding this comment.
this should resolve to
def validate("trento.checks.v1.FactsGathered", data),in this example for this event.
And all the supported schemas.
Very nice!
| @@ -0,0 +1,28 @@ | |||
| defmodule Wanda.Policy do | |||
There was a problem hiding this comment.
Just a reasoning point.
Could this Policy be something like a Processor|Injestor|SomethingThatProcessesIntegrationEvents? 🤔
I am not sure, will need to see how that evolves while adding more events.
There was a problem hiding this comment.
let's keep on the radar, i didn't came out with a proper name yet. maybe event handler?
This PR adds the setup to receive events and it adds the first handled event
FactsGatheredEventIt adds the basic setup for cloudevents and jsonschema validation that are going to be moved in https://github.com/trento-project/contracts ASAP.
The execution API now implements a behaviour, this simplify the DI and the mocking in the event handling logic so we do not rely on the GenServer/message passing while testing.
The event handling logic is responsibility of the
Wanda.Policymodule, in this PR theGenRMQ.Consumerimplementation is not tested but it could be easily tested with an integration test while we iterate.