Puppet PR Triage 2014-10-07

[Rob Reynolds]

Join us for the Puppet PR Triage starting in about 10 minutes at http://links.puppetlabs.com/pr-triage

--

Rob Reynolds

Developer, Puppet Labs

Join us at PuppetConf 2014, September 20-24 in San Francisco

[Rob Reynolds]

Puppet PR Triage Notes 2014-10-07

Recording: https://www.youtube.com/watch?v=R6iV-hv0d-w

Attending

Andy Parker (zaphod42), Britt Gresham (demophoon), Felix Frank (ffrank), Kylo Ginsberg (kylog), Rob Reynolds (ferventcoder), Will Hopper (whopper)

Summary

At the start of the triage Puppet had 63 open PRs, Facter had 5 open PRs, and Hiera had 2 open PRs.

New

Facter

798 - FACT-719 Fix invalid argument warning when reading dmi data

• Action: 10/07 - kylog waiting on contributor and will move forward

Puppet

3164 - PUP-3357 Purge unamed SSH Keys

• regression in Puppet 3.7.0
• generate implicit names for keys
• ffrank considering adding filepath in name as well
• ferventcoder asked about duplicates, ffrank said could take multiple converges
• ffrank mentioned using truly unique name (add path and/or index)
• kylog and ferventcoder agreeing this is a needed change
• How do we get Puppet to purge all keys related to an unnamed with just one go?
• Action: 10/07 - ffrank to add file name as part of key resource name, might be a week or so.

3146 - PUP-3244 ENC ignores missing directory environments

• ferventcoder concerned about double hit to same method that may be a perf penalty
• zaphod42 brought up a better way to get (get!) so that could really reduce boilerplate code
• zaphod42 brought up removal of elsif and check on a type if we are not going to use the type
• Action: 10/07 - demophoon to fix up PR

3167 - PUP-3373 (doc) Clarify options parameter docs for mount

• 10/07: ferventcoder noted fixes to grammar

Held Over

Facter

786 - FACT-680

• this matches what wikipedia mentions is how to detect link local
• mikaelsmith adding a cfact ticket to follow up
• This looks pretty good
• Action: 10/01 - waiting on contributor for new tests versus changing existing tests.
• 10/07 - ferventcoder pinged contributor

778: FACT-698 Add fips_enabled fact

• Josh adding notes to this PR and related PUP-3114 PR at 3044
• Uncertain of the value of the fact if we don't intend to merge PUP-3114
• Pinged Nick F on PUP-3114 for addressing this in docs
• 10/01 - still believe this is a documentation issue for explicit. If we fix the doc in Puppet, not sure if this is something to move forward on with facter.
• 10/07 - NO REVIEW
• Action: Review Next Triage

Hiera

213 - HI-183 - Support complex value lookups

• this is only additive
• Action: 10/01 - ferventcoder to look at this sprint
• 10/07 - NO REVIEW NECESSARY

209 - (maint) use a more recent version of beaker

• when ready to merge, we'll want to watch the tests knowing this just went in
• we may want to prefer an even more recent version of beaker
• Action: 10/01 - holding on this for now, will evaluate again later
• 10/07 - NO REVIEW

Puppet

3145: PUP-3341 - Puppet apply breaks when an ENC returns an environment

• This is similar to 3144 (includes commits from 3144), so could come after
• ffrank: This is a bit more invasive, could override what ENC would tell a node to do
• joshcooper: https://projects.puppetlabs.com/issues/3910 is the behavior we added in 3.x
• ffrank: Shouldn't puppet apply exclusively look local?
• should apply option have option to override environments?
• zaphod42: we've been using the general rule that if it is specified on command line, it should take precedence.
• kylog: what if they disagree (both manifest and an ENC)?
• zaphod42: command line versus config can be discerned, just not yet sure if it can be discerned at the right point
• Holding on this for now while we evaluate more of the implications - Let's start a puppet-dev thread
• 10/05 - https://groups.google.com/forum/#!msg/puppet-dev/l5CnnSvOpW0/3cl-kzaacJQJ (@ffrank to start thread)
• 10/07 - Closed by ffrank to retarget to master
• Action: 10/07 - ffrank retargeting against master

3127 - PUP-3355 - Remove pson document type

• joshcooper: I believe it is backwards compatible
• Action: 10/01 - Pulling into the next sprint
• 10/07 - NO REVIEW NECESSARY

3112 - PUP-3331 - Provide auto* relationships for types

• zaphod42: initial reaction on mailing list
• kylog will move it forward once specs are in
• 10/01 - waiting on contributor for specs
• Action: 10/07 - kylog or ferventcoder will pick this up and fixup - priority comes after 3.7.2 items

3107 - PUP-3319 - Retry send report if connection to master fails

• sets a limit of 4 tries, but adds a retries parameter
• zaphod42: this may catch system exit as well. That can be fixed.
• zaphod42: it would be good to have a test, so if report does fail
• much discussion and looking at removed retryaction.rb.
• we'd want to clean this up and bring it back in as retry_action.rb
• add blocks that could be passed to retry_action.
• Action: 10/01 - We are going to fix up retryaction and then reevaluate this PR.
• 10/07 - NO REVEW - Holding on this until we have retryaction updated.

3027: PUP-3232 Prevent group duplication when group list created from multiple sources

• Need JIRA ticket, and some sort of tests
• As of 9/10, Kylo to notify contributor
• Specs and ticket added, but specs failing
• 10/01 - followup next meeting (skipped)
• Action: 10/07 - kylog to take this and fixup

2919: PUP-2995 Proposal for processing an external trusted OID yaml file

• JIRA issue has been filed for this pull request
• Needs spec tests, needs refining of the implementation. We'll continue work on this with the contributor.
• Check in with Adrien tomorrow 8/14 about picking this up now that he's rotated off of community role
• JoshP to check in on the settings behavior, given we don't want to use a class variable to track init state (and have several different hook options available for determining when settings can be changed)
• As of 9/3, Ethan asked for contributor clarification on implementation
• As of 9/10, much discussion on this particular issue -- we're uncertain of the lifecycle implications. Adrien suggested an alternative to hooks, and JoshC had some feedback on code.
• 9/30 Kylo pinged contributor.
• 10/01 - waiting on contributor
• 10/07 - Travis CI failed, kylog kicked back off
• 10/07 - ferventcoder added code review comments to current changes
• 10/07 - zaphod42 brought up ParseError being possibly used for something that it should not be.
• 10/07 - Person taking this should investigate what the best way to do that will be and fixing up.
• 10/07 - ffrank asked contributor about a possible better approach to the YAML format
• Action: 10/07 - Wait on contributor for fixup

2649: Add slot support for portage package provider.

• Duplicates GH-2779, decision is to ping both contributors to see if they can coalesce their individual PRs into a single branch. Finch commenting this week.
• GH-2779 hasn't seen updates, closing that one in favor of this one. There's some concern about possibly using :as a special value in package resource titles but we don't want to go down that route.
• Pinged Adrien for a final call on this now that the specs are updated
• Ethan to look at next week at some point
• As of 9/3, Kylo asking contributor for a ticket, will revisit next week
• As of 9/10, Kylo following up on following up
• 9/30 - Kylo pinged contributor for ticket.
• Action: 10/01 - waiting on contributor
• 10/07 - NO REVIEW

2770: PUP-1537 Allow version and held states of a package to specified as separate properties

• This is a very big and sticky issue, and might be worth promoting into the platform team because of the wide ranging ramifications
• Plan is to discuss how we promote issues like this, and respond to the contributor with our next steps forward
• Pending contributor response, willing to wait on this since we're asking a lot of the contributor
• Will need to talk with Adrien tomorrow and figure out how to hand this off.
• Didn't have time to look at on 8/27 -- major PR that has breaking changes and could require work. Would need to be targeted at Puppet 4 at the very least, but this one is tricky. Will try to investigate at next PR meeting.
• As of 9/3, punting this one a week, since it's a BIG change
• As of 9/10, probably punting to post-PuppetConf - too big for now
• Action: 10/01 - This needs a ticket number and should probably be one of the workshopped PRs kylog had mentioned in his email on triages. That way we could concentrate on this one. The commits would need fixed up to include ticket number
• 10/07 - ferventcoder pinged contributor about failing specs

Merged or Closed Prior to Triage

Facter

787 - FACT-706 - Fixing xenstore-read errors

• looks good, pulling in
• 10/01 - Merged by whopper

777: FACT-695 Return correct data types

• Putting on hold until 10/01 for additional discussion
• 10/01 - dalen: should this be part of Facter 3?
• Action: 10/01 - Kylo to scrub through and move forward week of 10/01.
• 10/06 - Merged by kylog

Hiera

208 - (maint) inform which key caused type mismatch

• looks good - commit message needs fixed up
• 10/02 - kylog fixed up and merged updated PR #216

Puppet

3144: PUP-3258 - Fix ENC issue with puppet apply

• 10/01 - zaphod42 to evaluate. Pulling into this upcoming sprint
• 10/02 - Merged by zaphod42