[Puppet-dev] Puppet PR Traige Notes 2014-09-10

[Ethan Brown]

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=RjBXOF-BrOE


Attending: Ethan, Kylo, Whopper, EricT, FelixFrank (community)

New:

Facter

No new Facter tickets reviewed on 9/10.

Puppet:

2990: (maint) better error message when a relationship can't be established
- As of 9/10, Ethan left comment with contributor about commit msg

3002: (#21272) Refactor created directory from Puppet module generate
- As of 9/10, pinged Anderson /brandon from modules team

3004: There's no way 'bsd' provider will work on OpenBSD
- As of 9/10, Kylo to follow up with jasper about creating Jira ticket and
following contribution guidelines.

3013: PUP-3137 document updated
- As of 9/10, pinged Carthik to update commit msg, NickF review

3015: Remove "{}" from PUPPETMASTER_EXTRA_OPTS and PUPPET_EXTRA_OPTS
- As of 9/10,Kylo to ping Haus and have contributor open ticket

3003: PUP-3128 enabled pacman as default provider for manjarolinux
- As of 9/10, Ethan merged

3027: 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

Held Over

Facter:

None reviewed on 9/10 - all held over.

774: Detect KVM even when generic CPU model name is used
- Asked contributor for some minor fix ups, addition of new ticket to FACTER

777: FACT-695 Return correct data types
- Putting on hold until 9/10 for additional discussion

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

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

Puppet:

2986: (PUP-3119 )Added yaml output to puppet resource
- Pinged Henrik and Eric to get their opinions; Eric is discussing
it with contributor in the PR.
- Ticket has been created, some discussion has moved forward, it
seems like this is a useful operation. Still no tests, code requires
some additional cleanup.
- Kylo contacting contributor with additional details about
maintenance commits / PRs
- 9/10 - Kylo to squash commits / merge

2866 PUP-1244 puppet4
- Switches the yum provider to use RPM version comparison instead of
semantic version comparison
- This might be breaking but is more correct and has been requested
since Puppet 0.24, document as a breaking change in Puppet 4
- Commit messages need to be fixed up, Andy to handle
- Has an error; @jantman will be looking into it this week; check
back next week.
- Still waiting on contributor for changes; Joshua to ping
- Jason has updated the failing case JoshP saw; Joshua to test again
- Tests out fine, and has been squashed, but seeing a failure on
1.8.7; JoshP will look at it
- As of 8/27, JoshP still investigating failure on 1.8.7
- As of 9/3, Ethan rerunning job on Travis, will merge if it passes
- As of 9/10, this needs to have some miscellaneous white space / code cleanup
performed, remove use of and in favor of &&, etc - Kylo to cleanpu


2952: PUP-3038 Cache search_directories
- Ongoing discussion about unexpected impact from this change
- Breaking specs
- Commit message neds updating to provide ticket # and information
about the change
- Felix also wondered how this related to PUP-1592 and has updated
PUP-3038 to link that
- Ethan added note that PUP-3032 might address this already, and
asked contributor to measure impact on his environment. It's possible
this ticket might be closed if the perf gains with the work on
PUP-3032 are sufficient.
- As of 9/3, contributor has not mentioned if we fixed his issue with the
always_cache_features setting, will wait another week for response
before closing
- As of 9/10, closed


2951: PUP-3038 Cache typeloader misses
- Same PUP-3038 ticket as 2952, similar concerns
- This is something that seems like a reasonable idea, but could
have some difficult to debug failure scenarios (when changing
environments for instance), that we're uncertain of at this point.
- Suggest we use a sentinel value for a TypeNotFound situation,
rather than relying on nil (even if it adds more code to the
PR, this will be more clear)
- We'd like to evaluate the concept for Puppet 4, but making this
part of 3.7 is not going to happen right now.
- JoshP to discuss Henrik comments about viability of this approach.
- As of 9/3, pinged Henrik / Josh about closing this.
- As of 9/10, closed

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. Ethan to follow up with Adrien to see what we
might want to do to address

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

2683: Add more config manipulation options
- Needs a JIRA issue and tests are failing
- Would be a good fix to have, if contributor doesn't get back to us
might be worthwhile to fixup ourselves.
- Adrien asking for a Ticket so we can pull it onto community board
and tackle it.
- No response/no ticket, Joshua to create ticket
- Discussion that we should preserve the current behavior of `puppet
config set prop ''` and you should use `puppet config del` to remove
the setting and restore the default. JoshP to continue work.
- Comments from JoshP from 19 days ago have not been addressed as of 8/27
- As of 9/3, waiting for a response from Ben
- As of 9/10, waiting for a response from Ben still, will close on 9/17 if now

2770: 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

2870 (PUP-2526) Make the puppet agent retry failed requests.
- Issue in commit message references projects.puppetlabs.com ticket,
actual issue is https://tickets.puppetlabs.com/browse/PUP-2526
- There are a lot of cases where HTTP retries aren't meaningful
(catalog retrieval, report submission)
- We haven't been able to come to an agreement with the contributor
about what requests should be retried and how, and we might not have
the resources to make the changes ourselves.
- Felix commented in the ticket, some discussion is happening there
- Waiting to see if we can come to some agreement in the ticket as
to the direction of the change.
- As of 8/27, still stewing on this one
- As of 9/3, Kylo to ping contributors in ticket, still need to
discuss further
- As of 9/10, closed the ticket based on lack of feedback / consensus

2869 PUP-3010 Allow Upstart jobs on Amazon Linux
- Ticket filed, issues pending on the pull request
- Waiting on contributor feedback
- Needs a spec and an updated commit message
- Ethan pinged contributor on 8/27
- Ethan to verify again this is ready, and will merge to master once
3.7 has shipped
- As of 9/3, still waiting on Puppet 3.7 to ship
- As of 9/10, Ethan will fixup / merge this commit

2820: (PUP-2867) Support ~ in file paths, allows to deploy to ~<user>
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.

2614: (PUP-2452) Refresh support for Package type
- Looks reasonable.
- There is a question from the contributor about if there needs to
be something changed to use this.
- The answer to the question is, "yes, having a use of this is ideal".
- Adrien noted that using the :methods system for the feature is
hard to control. Better is to require that the provider just
explicitly declare the feature, which it already does.
- Another concern came up about getting refresh events that will
cause unintended reinstalls. Such a case is when a class contains a
package and is the target of a notify.
- This is a larger change to functionality that we need to bring up
on puppet-dev to figure out how to deal with the unintended
consequences of adding this functionality.
- Adrien to file a ticket on finer grained refresh handling and
start a thread on puppet-dev to come to a decision about how to better
control refresh events.
- Some discussion taking place in the PR, waiting on the contributor
for an updated opt-in approach
- As of 8/27, contributor still working on PR - will re-eval next week
- As of 9/3, contributor has made reasonable changes / provided tests,
Ethan to pull in / merge once Puppet 3.7 has shipped. Likely rename
refreshable to reinstallable.
- As of 9/10, Ethan to review locally / merge for next week



--
--
Ethan Brown
et...@puppetlabs.com
Senior Software Engineer

Join us at PuppetConf 2014, September 20-24 in San Francisco
Register by September 8th to take advantage of the Final Countdown —save $149!