-
Notifications
You must be signed in to change notification settings - Fork 15
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
Ecto decode cloud discovery #582
Conversation
ec988b6
to
1dd2bb6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
provider_data: | ||
case metadata do | ||
%{ | ||
compute: %{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we have a function here instead of a case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have something like:
defp build_update_provider_command(agent_id, %CloudDiscoveryPayload{
provider: :azure,
metadata: metadata
}) do
...
To handle the different provider overtime, instesad of the case
option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copy that 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @dottorblaster !
I agree on the comment given by @fabriziosestito on splitting the metadata building process by providers
lib/trento/application/integration/discovery/payloads/cloud_discovery_payload.ex
Show resolved
Hide resolved
lib/trento/application/integration/discovery/payloads/cloud_discovery_payload.ex
Show resolved
Hide resolved
lib/trento/application/integration/discovery/payloads/cloud_discovery_payload.ex
Show resolved
Hide resolved
provider_data: | ||
case metadata do | ||
%{ | ||
compute: %{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have something like:
defp build_update_provider_command(agent_id, %CloudDiscoveryPayload{
provider: :azure,
metadata: metadata
}) do
...
To handle the different provider overtime, instesad of the case
option
1dd2bb6
to
d7ab945
Compare
Decoding cloud discovery payload using Ecto.