-
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
Add a scheduled software updates discovery job #2376
Conversation
0d30751
to
2afc779
Compare
2afc779
to
39726f6
Compare
98654d7
to
dcf5f14
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.
beautiful 👌
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 with a nitpick
def discover_software_updates, | ||
do: | ||
{:ok, | ||
Hosts.get_all_hosts() | ||
|> Enum.map(&discover_host_software_updates/1) | ||
|> Enum.split_with(fn | ||
{:ok, _, :discovered, _, _} -> true | ||
_ -> false | ||
end)} |
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.
Can't you just return []
over an erroring discovery? you can keep the logging and you just have to Enum.flatten
this list instead of playing with tagged tuples.
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.
When testing I want to rely on the output of the function, rather than logging 😅 That's the reason behind it.
We could alternatively return just one list with all the discovery results, successful and errored. However we'd need to do filtering of successful vs errored results in the test to make meaningful assertions. We get that distinction for free by splitting here. I find it convenient.
I simplified a bit the tuple returned in success case from {:ok, host_id, :discovered, system_id, relevant_patches}
to {:ok, host_id, system_id, relevant_patches}
👍
Description
This PR adds the capability to discover software updates for all the registered hosts in trento.
This has been scheduled to happen every 12 hours.
How was this tested?
Automated tests.