Puppet Core Community Pull Request Triage 10-01-2014
Rob Reynolds
Rob Reynolds
Puppet PR Triage Notes 2014-10-01
FYI, this weeks triage was hosted as a 'Hangout on the Air', and therefore a recording has been automatically published to YouTube. For those unavailable to attend, but interested in reviewing what was discussed, the video is available at:
https://www.youtube.com/watch?v=oyxYiOPTQvI
Attending
Josh Cooper (joshcooper), Erik Dalén (dalen), Kylo Ginsberg (kylog), Will Hopper (whopper), Felix Frank (ffrank), Michael Smith (mikaelsmith), Rob Reynolds (ferventcoder), Andrew Parker (zaphod42)
New:
Hiera
208 - (maint) inform which key caused type mismatch
- looks good - commit message needs fixed up
- Action: 10/01 - ferventcoder to do
213 - HI-183 - Support complex value lookups
- this is only additive
- Action: 10/01 - ferventcoder to look at this sprint
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
Facter
787 - FACT-706 - Fixing xenstore-read errors
- looks good, pulling in
- Action: Merged by whopper on 10/01
- 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.
Puppet:
3144: PUP-3258 - Fix ENC issue with puppet apply
- Action: 10/01 - zaphod42 to evaluate. Pulling into this upcoming sprint
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
- Action: Holding on this for now while we evaluate more of the implications - Let's start a puppet-dev thread
- Action: 10/01 - FIXME WITH LINK (@ffrank to start thread)
3127 - PUP-3355 - Remove pson document type
- joshcooper: I believe it is backwards compatible
- Action: 10/01 - Pulling into the next sprint
3112 - PUP-3331 - Provide auto* relationships for types
- zaphod42: initial reaction on mailing list
- kylog will move it forward once specs are in
- Action: 10/01 - waiting on contributor for specs
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.
Held Over
Facter:
777: FACT-695 Return correct data types
- Putting on hold until 10/01 for additional discussion
- 10/1 - dalen: should this be part of Facter 3?
- Action: 10/01 - Kylo to scrub through and move forward week of 10/01.
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
- Action: 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.
Puppet:
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
- Action: 10/01 - followup next meeting (skipped)
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.
- Action: 10/01 - waiting on contributor
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
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.
Merged or Closed Prior to Triage
Facter:
774/795: FACT-700 Detect KVM even when generic CPU model name is used
- Merged 9/30 in 795 (same as PR 774 but with squashed commits)
769: FACT-680
- Ethan to notify contributor to add test, update commit msg
- Still waiting on contributor response as of 9/3, Kylo to ping contributor
- Contributor closed on 9/11
Puppet:
2990: (maint) better error message when a relationship can't be established
- Merged 9/10
3002: (#21272) Refactor created directory from Puppet module generate
- Merged 9/12
3004: There's no way 'bsd' provider will work on OpenBSD
- Merged 9/16
3013: PUP-3137 document updated
- Merged 9/30
3015: Remove "{}" from PUPPETMASTER_EXTRA_OPTS and PUPPET_EXTRA_OPTS
- Merged 9/10
3003: PUP-3128 enabled pacman as default provider for manjarolinux
- Merged 9/10
2986/3077: (PUP-3119 )Added yaml output to puppet resource
- Merged 9/10 in 3077 (same as PR 2986 but with squashed commits)
2866/3078 PUP-1244 Fix yum/rpm version comparison
- Merged 9/10 in 3078 (same as PR 2866 plus squash/whitespace changes)
2869 PUP-3010 Allow Upstart jobs on Amazon Linux
- Merged 9/30
2820: (PUP-2867) Support ~ in file paths, allows to deploy to ~ directories
- Already in discussion on puppet-dev
- Tests are failing, commits need squashing; waiting for contributor response
- What happens when the path starts with
~/- do we expand to the current user? - Felix clarified the above issue; waiting on contributor
- As of 8/27, debate ongoing - will revisit in next PR triage.
- As of 9/3, Kylo leaving comments on this ticket. Current status is that we remain unconvinced of this being necessarily useful for everyone.
- As of 9/10, waiting on contributor response. Will give it one more week before we close the PR.
- Closed 9/30.
2614/3137: (PUP-2452) Refresh support for Package type
- Merged 9/30 in 3137 (same as PR 2614 plus a docs and param name change)