Puppet PR Triage 2014-10-15

[Henrik Lindberg]

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

- henrik
--

Visit my Blog "Puppet on the Edge"
http://puppet-on-the-edge.blogspot.se/

[Rob Reynolds]

Notes are also located at https://github.com/puppet-community/community-triage/blob/master/core/notes/2014-10-15.md

Puppet PR Triage Notes 2014-10-15

Recording: https://www.youtube.com/watch?v=of-AY9fj4wQ

Attending

Britt Gresham (demophoon), Daniele Sluijters (daenney), Eric Thompson (er0ck), Felix Frank (ffrank), Henrik Lindberg (hlindberg), Jeff Palmer (palmerit), Josh Cooper (joshcooper), Kylo Ginsber (kylog), Leonardo Mello (lmello), Rob Reynolds (ferventcoder), Will Hopper (whopper)

Summary

At the start of the triage Puppet had 58 open PRs, Facter had 5 open PRs, and Hiera had 1 open PRs.

New

Puppet

3196 - PUP-3470 - Fix Ruby 1.8.7 syntax error in test suite

• we are dropping 1.8.7 in 4.x series, so it won't support this
• if we bring this in, we should target stable instead so it will go in 3.7.x series
• 10/15 - hlindberg closing as won't fix as it won't be an effective change for 4.0.x - and it works on newer versions of 1.8.7-p370+

3166 - PUP-3393 Minor typo fix in relationship_spec.rb

• ffrank, ferventcoder are thumbs up
• 10/15 - ferventcoder merged to master

3178 - PUP-2923 Add support for assume yes to the yum provider

• ferventcoder pinged a couple of folks and haus responded that this looks good and works all the way back to EL4.
• Action: 10/15 - ferventcoder to cleanup and merge - to evaluate PUP-778 as well

3124 - PUP-3346 Don't allow insecure SSL connections w/curl

• ffrank noted that the -k is there from the beginning of time when this provider was added
• Action: 10/15 - kylog left a note on ticket asking if this would be an issue specifically with OS X as it appears this is what this addresses

3175 - PUP-3462 Hiera scope: add the key 'calling_class_path'

• this is with windows not being able to use ':' as part of the filename
• 10/15 - kylog retargeting to 4.0
• Action: 10/15 - ferventcoder to take for followup

3005: PUP-2289 Fix host type and provider

• 10/15 - kylog mentioned operators need cleaned up
• 10/15 - joshcooper mentioned types of errors you should expect
• would this be able to handle other issues? lmello - it handles incomplete entries and new line in resource attributes. If it doesn't understand a line it will treat it as text.
• lmello - should send warnings when it finds incomplete lines? ferventcoder leans towards debug messages, kylog, hlindberg lean 50/50 on this - it's a bit of a gray area - it depends on if Puppet handles it gracefully
• Action: 10/15 - lmello to fix up

3165 - PUP-1077 Remove Nagios from core

• daenney was curious where this was and if there was anything to do
• kylog said we are about a week or two out on this - working on logistics
• Acton: 10/15 - holding while we work out logistics

Held Over

Facter

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

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

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, contributor is on vacation for a couple of weeks (noted in JIRA ticket)

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
• 10/15 - ferventcoder pinged NickF/Joshcooper on the ticket about whether a decision has been made

Hiera

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

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?
• 10/07 - ffrank to add file name as part of key resource name, might be a week or so.
• 10/15 - kylog, ferventcoder thumbs up, hlindberg, joshcooper expressed concerns about uniqueness
• 10/15 - ffrank to fixup, kylog to merge
• 10/15 - kylog merged

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
• 10/15 - discussion is still pending

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
• 10/15 - kylog to look at this week

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
• 10/07 - Wait on contributor for fixup
• 10/15 - looks better, ferventcoder asked about ParseError - hlindberg mentioned this should not be ParseError - but why pick on contributor for fixing this up as we've already changed course a few times
• 10/15 - hlindberg fixed up and merged

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
• 10/15 - 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

Hiera

213 - HI-183 - Support complex value lookups

• this is only additive
• 10/01 - ferventcoder to look at this sprint
• 10/09 - ferventcoder merged into master

Puppet

3127 - PUP-3355 - Remove pson document type

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

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
• 10/07 - demophoon to fix up PR
• 10/10 - zaphod42 merged this (and we followed up with a couple fixups)

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

• 10/07 - ferventcoder noted fixes to grammar
• 10/09 - kylog merged

 

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/m1m891%2446c%241%40ger.gmane.org.
For more options, visit https://groups.google.com/d/optout.

--

Rob Reynolds

Developer, Puppet Labs