Bug #4573
closed
Remove unused variables ADMIN, POLICYCHILDREN and CHILDRENID from "common" system technique metadata
Added by François ARMAND over 10 years ago. Updated over 10 years ago.
Description
I'm not sur about the version for which this is the case, it should match the change done for relay servers.
I'm not sur about ADMIN, I believe it was already removed in 2.10.
Updated by Nicolas CHARLES over 10 years ago
POLICYCHILDREN and CHILDRENID were removed in 2.8, ADMIN in 2.10.
I'm openin another ticket for ADMIN
Updated by Nicolas CHARLES over 10 years ago
- Status changed from 8 to Pending technical review
- Assignee changed from Nicolas CHARLES to Jonathan CLARKE
- Pull Request set to https://github.com/Normation/rudder-techniques/pull/312
Updated by Nicolas CHARLES over 10 years ago
- Status changed from Pending technical review to In progress
ha, it is a bit more complex than that, as it fails at deployment
Updated by Nicolas CHARLES over 10 years ago
- Assignee changed from Jonathan CLARKE to Nicolas CHARLES
Updated by Nicolas CHARLES over 10 years ago
- Status changed from In progress to Pending technical review
- Assignee changed from Nicolas CHARLES to Jonathan CLARKE
Ok, it doesn't fail, it just complains in the log that some variables cannot be stored, but it works
Updated by Jonathan CLARKE over 10 years ago
- Status changed from Pending technical review to Discussion
- Assignee changed from Jonathan CLARKE to François ARMAND
I don't understand this ticket. Removing stuff is not a bug. What is the actual bug (what doesn't work or is broken here?). Please clarify.
Updated by François ARMAND over 10 years ago
Having dead code is a bug:
- it may be call in unwanted or unknown way, leading to bigger problem (security pb) and/or extremely hard bug to reproduce
- it may send bad message to future mainteners, thinking that something is used but is actually not.
That's of course generalities, but in that precise case, these variables were still present in technique metadata but not used for the promise generation, so we add a desynchronisation between technique metadata and actual variable, and we were paying a non negligible processing time for them for nothing.
Moreover, they were the only remaining use cases of the ${rudder.ruleid.target.XXX} parameter kind, blocking a possible major simplification (see #4585)
Updated by François ARMAND over 10 years ago
- Status changed from Discussion to Pending technical review
- Assignee changed from François ARMAND to Jonathan CLARKE
Updated by Jonathan CLARKE over 10 years ago
- Status changed from Pending technical review to Discussion
- Assignee changed from Jonathan CLARKE to François ARMAND
François ARMAND wrote:
Having dead code is a bug:
- it may be call in unwanted or unknown way, leading to bigger problem (security pb) and/or extremely hard bug to reproduce
- it may send bad message to future mainteners, thinking that something is used but is actually not.That's of course generalities, but in that precise case, these variables were still present in technique metadata but not used for the promise generation, so we add a desynchronisation between technique metadata and actual variable, and we were paying a non negligible processing time for them for nothing.
Moreover, they were the only remaining use cases of the ${rudder.ruleid.target.XXX} parameter kind, blocking a possible major simplification (see #4585)
Thanks for the clarification.
I agree this needs removing, however, in the interest of stability, we must avoid making unnecessary changes, and in particular any changes that don't fix a problem suffered by users in released branches. Can we retarget this to 2.10?
Updated by François ARMAND over 10 years ago
- Status changed from Discussion to Pending technical review
- Assignee changed from François ARMAND to Jonathan CLARKE
Well, they should have been removed along with the introduction of MANAGED_NODES_NAME and MANAGED_NODES_ID in #4033 (so in 2.8), but I'm ok to remove them only in 2.10.
Updated by Jonathan CLARKE over 10 years ago
- Status changed from Pending technical review to Discussion
- Assignee changed from Jonathan CLARKE to Nicolas CHARLES
François ARMAND wrote:
Well, they should have been removed along with the introduction of MANAGED_NODES_NAME and MANAGED_NODES_ID in #4033 (so in 2.8), but I'm ok to remove them only in 2.10.
Great. Assigning to Nicolas to retarget the PR.
Updated by Jonathan CLARKE over 10 years ago
François ARMAND wrote:
Having dead code is a bug:
Oh, and to respond to this, let me clarify what my problem here is: maybe having dead code is a bug, but the title of this ticket never mentions dead code. It is expressed exactly like a "Let's remove some stuff" ticket which sounds scary, whereas it should be more like a "Let's clean up this unused stuff" which sounds good.
Titles matter. A lot.
Updated by François ARMAND over 10 years ago
- Target version changed from 2.8.4 to 2.10.0~beta1
Updated by François ARMAND over 10 years ago
- Status changed from Discussion to In progress
Nico, could you retarget the associated PR (and other for the children) to 2.10 (given all the discussion above ?)
Thanks !
Updated by Nicolas CHARLES over 10 years ago
- Pull Request changed from https://github.com/Normation/rudder-techniques/pull/312 to https://github.com/Normation/rudder-techniques/pull/317
retargetted to master:
https://github.com/Normation/rudder-techniques/pull/317
Updated by Jonathan CLARKE over 10 years ago
- Subject changed from Remove ADMIN, POLICYCHILDREN and CHILDRENID from "common" system technique metadata to Remove unusued variables ADMIN, POLICYCHILDREN and CHILDRENID from "common" system technique metadata
Clarifying the title to make it actually sound like a bug!
Updated by Nicolas CHARLES over 10 years ago
- Status changed from In progress to Pending release
- % Done changed from 67 to 100
Applied in changeset policy-templates:commit:6f23559579946857d0bda5fd6c252e926f4c264d.
Updated by Anonymous over 10 years ago
Applied in changeset policy-templates:commit:cec1a3bd22ad95414386b2b935fdf1a8dc37eb16.
Updated by Vincent MEMBRÉ over 10 years ago
- Subject changed from Remove unusued variables ADMIN, POLICYCHILDREN and CHILDRENID from "common" system technique metadata to Remove unused variables ADMIN, POLICYCHILDREN and CHILDRENID from "common" system technique metadata
Updated by Vincent MEMBRÉ over 10 years ago
- Status changed from Pending release to Released
This bug has been fixed in Rudder 2.10.0~beta1, which was released today.
Check out:
The release announcement: http://www.rudder-project.org/pipermail/rudder-announce/2014-March/000084.html
The full ChangeLog: http://www.rudder-project.org/foswiki/bin/view/System/Documentation:ChangeLog210
Download information: https://www.rudder-project.org/site/get-rudder/downloads/