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 #20747: Use a unique id to identify reports #4167

Conversation

VinceMacBuche
Copy link
Member


def jsonComponentExpectedReport(c : ExpectedValue): JValue = {
c match {
case v : ExpectedValueId =>
Copy link
Member

Choose a reason for hiding this comment

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

we will need to still be able to deserialize old data structure to be able to have compliance for old explect reports

@@ -583,7 +583,7 @@ class ClassicTechniqueWriter(basePath : String, parameterTypeService: ParameterT
def reportingContext(methodCall: MethodCall, classParameterValue: String ) = {
val component = escapeCFEngineString(methodCall.component)
val value = escapeCFEngineString(classParameterValue)
s"""_method_reporting_context("${component}", "${value}")"""
s"""_method_reporting_context("${component}", "${value}","${methodCall.id}")"""
Copy link
Member

Choose a reason for hiding this comment

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

we need to duplicate the method to avoid breaking stuff during upgrade

Copy link
Member

Choose a reason for hiding this comment

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

and we need the matching PR in ncf before merging this one

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 that will be a duplicated method. I need to validate the POC first.

@VinceMacBuche VinceMacBuche added the WIP Use that label for a Work In Progress PR that must not be merged yet label Feb 15, 2022
@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

@VinceMacBuche
Copy link
Member Author

PR updated with a new commit

@fanf fanf added Ready for merge and removed WIP Use that label for a Work In Progress PR that must not be merged yet Trigger test labels Feb 21, 2022
@fanf
Copy link
Member

fanf commented Feb 21, 2022

OK, squash merging this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants