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

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting #3650

Conversation

VinceMacBuche
Copy link
Member

@VinceMacBuche VinceMacBuche requested a review from fanf May 27, 2021 15:05
@VinceMacBuche VinceMacBuche force-pushed the ust_19323/be_able_to_group_reporting_and_methods_so_that_we_have_clearer_techniques_and_a_better_reporting branch from 0863f40 to bb47760 Compare May 27, 2021 15:07
@VinceMacBuche VinceMacBuche force-pushed the ust_19323/be_able_to_group_reporting_and_methods_so_that_we_have_clearer_techniques_and_a_better_reporting branch from bb47760 to 985c3a1 Compare May 27, 2021 15:12
@VinceMacBuche
Copy link
Member Author

Commit modified

Copy link
Member

@fanf fanf left a comment

Choose a reason for hiding this comment

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

WIP, fastly broken

@@ -437,3 +436,20 @@ object DisplayPriority {
}
}
}

sealed trait CompositionRule
case object WorstReport extends CompositionRule
Copy link
Member

Choose a reason for hiding this comment

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

can you put the cases in the object and mark them final (it's the conventional pattern used elsewhere)

|> appendChild cloneIcon
removeIcon = element "i" |> addClass "fa fa-times-circle"
removeButton = element "button"
|> addClass "text-danger method-action tooltip-bs"
Copy link
Member

Choose a reason for hiding this comment

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

missing title

Copy link
Member

Choose a reason for hiding this comment

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

The remove button does not work for components in groups

@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

10 similar comments
@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

@VinceMacBuche VinceMacBuche force-pushed the ust_19323/be_able_to_group_reporting_and_methods_so_that_we_have_clearer_techniques_and_a_better_reporting branch from 05700ee to f16781f Compare July 2, 2021 13:50
@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

5 similar comments
@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

Copy link
Member

@fanf fanf left a comment

Choose a reason for hiding this comment

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

Reviewed ~1/3 :) Some tiyn things to update for now, overall very good

case WorstReport =>
ComplianceLevel.compute(ReportType.getWorseType(subComponents.map(_.status)) :: Nil)
case SumReport => ComplianceLevel.sum(subComponents.map(_.compliance))
case FocusReport(component) => ComplianceLevel.sum(findChildren(component).map(_.compliance))
Copy link
Member

Choose a reason for hiding this comment

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

I think that if the component is not found, it should not be 0 or whatever ComplianceLevel.sum(Nil) does, but it should be a missing report error.

componentName : String
//only one ComponentValueStatusReport by value
//only one ComponentValueStatusReport by valuex.
Copy link
Member

Choose a reason for hiding this comment

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

typo

case _ => Left(Unexpected(s"Value '${value}' is not a valid reporting composition rule."))
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer that each ReportingLogic has a name attribute, and that name is used everywhere when the string is needed. If we want to ensure consistency.
So something like:

sealed  trait ReportingLogic { def name: Sring}
final case object WorstReport extends ReportingLogic { val name = "worst" }
final case class FocusReport(component : String) extends ReportingLogic { val name = "focus:${component}" }
etc

import ReportingLogic._
(("componentName" -> c.componentName)
~ ("composition" -> (c.reportingLogic match {
case WorstReport => "worst"
Copy link
Member

Choose a reason for hiding this comment

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

c.reportingLogic.name

reportingLogic match {
case WorstReport => ("type"-> "worst")
case SumReport => ("type"-> "sum")
case FocusReport(component) => ("type" -> "focus") ~ ("value" -> component)
Copy link
Member

Choose a reason for hiding this comment

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

ok, so you need to split name and serialize in ReportingLogic (with by default val serialize = name and for focus report something different)

val reportingLogic = block.reportingLogic match {
case WorstReport => "worst"
case SumReport => "sum"
case FocusReport(component) => s"focus:${component}"
Copy link
Member

Choose a reason for hiding this comment

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

block.reportingLogic.serialize


val t5 = System.nanoTime
u4 += t5-t4


(missing, unexpected, expected)
( unexpected, expected)
Copy link
Member

Choose a reason for hiding this comment

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

I don't get why you don't have missing at all here: the case is still possible if some reports were lost (are more likely because of bad reporting logic somewhere)

Copy link
Member Author

Choose a reason for hiding this comment

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

They are already treated in another branch, i got duplicated missing if i keep it

Copy link
Member Author

Choose a reason for hiding this comment

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

yes the difference is here: https://github.com/Normation/rudder/pull/3650/files/b5ee112bc450916897f1e3cbd6a032ff6feb5b21#diff-32bcd76fdbbd93efed1323a8e48c79320607d7ef97094e819936a74e6fab88feR938

Previously we treated missing reports in its own place, so we were parsing all reports to check missing components so we produced our missing reports ,then when doing expected we were filtering missing key ( by using okKeys)

in expected we now look for all expected keys, and when treating a component if we don't have reports we produce a missing

So we were going through all expected reports twice so find all missing and now this is done in one go

@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

1 similar comment
@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

@Normation-Quality-Assistant
Copy link
Contributor

OK, squash merging this PR

… Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
…ock and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
…ponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
…iqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
…t et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
…omponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… fixup! Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… fixup! fixup! Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… fixup! fixup! fixup! Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… fixup! fixup! fixup! fixup! Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… fixup! fixup! fixup! fixup! fixup! Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… fixup! fixup! fixup! fixup! fixup! fixup! Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… fixup! fixup! fixup! fixup! fixup! fixup! fixup! Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
… fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Rename GroupComponent et UniqueComponent into Block and Value

Fixes #19323: Be able to group reporting and methods so that we have clearer techniques and a better reporting
@Normation-Quality-Assistant Normation-Quality-Assistant force-pushed the ust_19323/be_able_to_group_reporting_and_methods_so_that_we_have_clearer_techniques_and_a_better_reporting branch from ecca3be to f5e8a9f Compare July 17, 2021 10:05
@Normation-Quality-Assistant Normation-Quality-Assistant merged commit f5e8a9f into Normation:master Jul 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants