Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated apt::source to work with new versions of puppetlabs-apt #129

Closed
wants to merge 5 commits into from

Conversation

alejandrobednarik-olx
Copy link

No description provided.

@alejandrobednarik-olx
Copy link
Author

There are some tests that need to be updated also.

@juniorsysadmin
Copy link
Member

@alejandrobednarik-olx if you want to save time on this, I think I have done most of what's required in this branch: https://github.com/juniorsysadmin/puppetlabs-nodejs/tree/apt-2x-fixes . I was going to wait for the next release to the Forge (ETA a few weeks) before submitting a PR, but if you like I can submit it now for some feedback before then, or you can take bits and pieces and add it to this PR. Also see #126

@igalic
Copy link
Contributor

igalic commented May 4, 2015

we've got pretty much all tests failing, and i have no idea why.

@jmcclell
Copy link

jmcclell commented Jun 7, 2015

@igalic You're good to go. You forgot to update the spec to account for the new data structure and you haven't re-integrated with master.

Here is this PR + my changes as a diff:
puppet-community:master...jmcclell:apt-2.x-spec-update

And here are the build results from Travis from that branch:
https://travis-ci.org/jmcclell/puppet-nodejs/builds/65754591

@alejandrobednarik-olx Do you plan to update your PR? It seems silly for me to submit another when you can just bring my small changes in.

@juniorsysadmin
Copy link
Member

@jmcclell There's also #133 which I think is ready to go. The main difference between my branch and your branch seems to be the use of ensure_packages. I think ensure_packages is a better option to avoid duplicate package declarations. But if anyone has a better way, I'm all ears.

@jmcclell
Copy link

jmcclell commented Jun 7, 2015

@juniorsysadmin I believe this module already requires stdlib, so I think you're right. ensure_packages is probably the better option. The only thing I see in your PR is you forgot to replace include_src in the test description. Certainly minor.

I'm running this PR currently in my environment, but don't care which one of these two actually makes it into the official repo. Just hoping one does soon!

@jmcclell
Copy link

jmcclell commented Jun 7, 2015

It seems that the ordering is broken when using puppetlabs-apt 2.x, at least on Ubuntu 14.04.

The repo does come before the package, as is defined by the anchor points here, but the apt_update isn't triggering before running Package[nodejs], so I'm getting the old version on first-run. I'm not sure if this module should be responsible for making sure Package[nodejs] waits for Exec[apt_update] or if there's a bug in puppetlabs-apt that should be addressed.

It may also be broken in apt 1.x, but this is my first time using this and already was using 2.x

@juniorsysadmin
Copy link
Member

@jmcclell That issue is to do with the setting for $::apt::_update['frequency'] and is probably best left up to the user to set in apt, since it may affect performance in subsequent runs.

@juniorsysadmin
Copy link
Member

#133 has been merged, so closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants