Skip to content

Conversation

@fatmcgav
Copy link

@fatmcgav fatmcgav commented Nov 14, 2016

[WIP] Add support for puppet-archive download provider in addition to maestrodev-wget.

We currently use puppet-archive for the majority of our downloads, so rather than adding another download module to our site, lets add support for puppet-archive to this module :)

Raising early to get feedback on changes etc...

To-do:

  • Add unit tests
  • Add integration tests
  • Document functionality changes

@fatmcgav fatmcgav changed the title [WIP] Add support for 'puppet-archive' download provider in addition … [WIP] Add support for 'puppet-archive' download provider Nov 14, 2016
@tmclaugh
Copy link
Contributor

Please don't muddle this module with choose your own adventure resource types. If there's a need to use a different module then explain the issue. If the issue is specific to you then fork this rather than complicating the code for others. As far as I can tell the only issue is your site does not wish to add an additional dependency which IMHO is not worth the added complication for everyone else that uses or has to support this.

@kenbreeman
Copy link
Contributor

I appreciate the initiative and puppet-archive looks like a nice module with some decent features, but I agree with @tmclaugh that switching to this module doesn't add significant value to puppet-nexus.

@fatmcgav
Copy link
Author

@tmclaugh / @kenbreeman Thank-you for the quick responses.

My reasoning is above; as far as I'm concerned removing the need for another module just for downloading a file is sufficient. The pattern of supporting different 'download' providers is in use by several other modules that we consume from the forge, and works great.

The 'interface' changes for anyone consuming, or wanting to consume the module, is a single new class param, which defaults to the current behaviour of using maestrodev-wget.

Yes, I'd agree that there's slightly more complexity in the nexus::package class, but that's all self-contained and the code itself is pretty easy to read and understand...

@fatmcgav
Copy link
Author

Any appetite to revisit the above decision and add this additional functionality for the wider benefit?

@fatmcgav fatmcgav force-pushed the add_puppet-archive_support branch from f8665a4 to ec3c72a Compare May 16, 2018 13:42
@logichard
Copy link

Hello, I think you should just change download module to puppet-archive because maestrodev-wget is deprecated and author of maestrodev-wget recommended to use puppet-archive. @fatmcgav if u want i can make a PR with this change.

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