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

Change software_updates_discovery_health default value #2528

Merged
merged 3 commits into from
Apr 18, 2024

Conversation

nelsonkopliku
Copy link
Member

@nelsonkopliku nelsonkopliku commented Apr 17, 2024

Description

In this PR

  • the default state for the host's software_updates_discovery_health is changed from unknown to not_set (required in followup changes setting software_updates_discovery_health and consequently host's health to unknown on discovery errors)
  • the calculation of the resulting software_updates_discovery_health based on discovered patches is moved from the Host aggregate to the Discovery
  • SoftwareUpdatesDiscoveryCompleted is changed to SoftwareUpdatesHealthChanged carrying only the computed health

How was this tested?

Automated tests.

@nelsonkopliku nelsonkopliku force-pushed the refactor-software-updates-completion-command branch from ca3e8f2 to b741de3 Compare April 17, 2024 07:54
@nelsonkopliku nelsonkopliku self-assigned this Apr 17, 2024
@nelsonkopliku nelsonkopliku added enhancement New feature or request elixir Pull requests that update Elixir code env Create an ephimeral environment for the pr branch labels Apr 17, 2024
@nelsonkopliku nelsonkopliku marked this pull request as ready for review April 17, 2024 08:00
@nelsonkopliku nelsonkopliku changed the title wip Change software_updates_discovery_health default health Apr 17, 2024
@nelsonkopliku nelsonkopliku changed the title Change software_updates_discovery_health default health Change software_updates_discovery_health default value Apr 17, 2024
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.

Looking good!
My unique concern is that we don't check for the current software discovery health when we receive the command, because if the health is the same, we don't need to emit the event.
Besides that, (except no_set/unset term, which I hope @jamie-suse can help), everything looks good.

PD: Let's not merge this in a hurry. We need to see the currently changed things in demo (like tags, selected checks, etc), as we will need to recreate the db and event store

Type that represents the possible health values for the software updates discovery process.
"""

use Trento.Support.Enum, values: [:passing, :warning, :critical, :unknown, :not_set]
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe unset is more appropriate that not_set.
What does our anglo native @jamie-suse say hehe ?

lib/trento/hosts/host.ex Outdated Show resolved Hide resolved
@nelsonkopliku nelsonkopliku force-pushed the refactor-software-updates-completion-command branch from 91fe6d0 to 614c3bc Compare April 17, 2024 09:43
@nelsonkopliku nelsonkopliku force-pushed the refactor-software-updates-completion-command branch from 614c3bc to fd7e957 Compare April 17, 2024 09:45
@nelsonkopliku nelsonkopliku merged commit 3c97b6c into main Apr 18, 2024
26 checks passed
@nelsonkopliku nelsonkopliku deleted the refactor-software-updates-completion-command branch April 18, 2024 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
elixir Pull requests that update Elixir code enhancement New feature or request env Create an ephimeral environment for the pr branch
Development

Successfully merging this pull request may close these issues.

2 participants