make our type camptocamp/archive compatible #176

igalic · 2016-06-21T17:56:10Z

our type now accepts the same parameters as camptocamp/archive and processes them correctly.

igalic · 2016-06-21T18:24:56Z

lib/puppet/type/archive.rb

+    (this parameter is for camptocamp/archive compatibility).'
+    newvalues(/\b[0-9a-f]{5,128}\b/)
+    munge do |val|
+      resource[:checksum] = val


this munge needs to be handled differently

daenney · 2016-06-21T18:45:11Z

So what's the purpose of achieving compatibility? I thought the idea was to merge the modules?

igalic · 2016-06-21T18:59:02Z

currently, it's impossible to drop-in-replace either of the modules with the other
This patch serves as an attempt to get them a little bit closer.

igalic · 2016-06-21T19:14:58Z

i've now spec tested the first module against my branch…
@lesaux's puppet-kibana4

diff --git a/.fixtures.yml b/.fixtures.yml
index ad31c79..b32fd48 100644
--- a/.fixtures.yml
+++ b/.fixtures.yml
@@ -1,7 +1,9 @@
 fixtures:
   repositories:
      stdlib: "https://github.com/puppetlabs/puppetlabs-stdlib.git"
-     archive: "https://github.com/camptocamp/puppet-archive.git"
+     archive:
+       repo: "https://github.com/igalic/puppet-archive-1.git"
+       branch: "camptocamp-compat"
      apt: "https://github.com/puppetlabs/puppetlabs-apt.git"
   symlinks:
     'kibana4': "#{source_dir}"

works out perfectly fine… because it has no tests for archive _

lesaux · 2016-06-21T19:17:32Z

I've recently deprecated installing kibana4 from the tar.gz archive, since elastic.co is now maintaining packages.

igalic · 2016-06-21T19:19:02Z

well that's one problem solved… 1209012390439034 others to go ;)

igalic · 2016-07-06T23:37:02Z

this commit now adds archive::download

n.b.: i didn't copy camptocamp's archive::download spec tests if they had any :D
relatedly: it appears that archive::download is "private"

alexjfisher · 2016-07-07T08:43:36Z

I've been wondering whether we really want to clutter this module with compatibility types and parameters.

If we get the missing features implemented, might it be better to then start creating PRs against popular modules that use camptocamp/archive? It's even possible to make a module work with both camptocamp/archive and puppet/archive by using stdlib's load_module_metadata. That's what I've done in cpitman/puppet-database_schema#4

igalic · 2016-07-07T10:33:38Z

from Joe Duffy's blog about Midori (a research operating system)

Lets say that for any given 1 producer of state there are 10 consumers. Rather than having each of those 10 defend themselves against error conditions, we can push the responsibility back onto that 1 producer, and either require a single assertion that coerces the type, or even better, that the value is stored into the right type in the first place.

@alexjfisher what you're suggesting is "worse is better". i disagree.

jyaworski · 2016-07-07T11:49:12Z

I agree with @igalic. We want to fix the module, not the things that use it. Take the Linux stance: 'userspace is never the problem.' I'm for adding compatibility types. We can remove them in a major release later.

alexjfisher · 2016-07-07T15:43:54Z

Fair enough. Thought it was worth asking anyway. It might still be interesting to know how many modules are using camptocamp/archive. I wonder if I can use the forge API to tell me...

alexjfisher · 2016-07-07T16:36:40Z

... turns out, I can ;)
These are the forge modules that list camptocamp/archive as a dependency in their latest release.
Ordered by total downloads across all releases.

[{:module=>"rtyler-jenkins", :downloads=>253718},
 {:module=>"bfraser-grafana", :downloads=>47516},
 {:module=>"camptocamp-tomcat", :downloads=>26947},
 {:module=>"tohuwabohu-duplicity", :downloads=>8957},
 {:module=>"tohuwabohu-drupal", :downloads=>6795},
 {:module=>"pennycoders-marathon", :downloads=>6392},
 {:module=>"jschilperoord-papertrail", :downloads=>6324},
 {:module=>"pennycoders-zookeeper", :downloads=>6078},
 {:module=>"pennycoders-mesos", :downloads=>5148},
 {:module=>"nrvale0-google_cloud_sdk", :downloads=>4108},
 {:module=>"cpitman-database_schema", :downloads=>4020},
 {:module=>"tohuwabohu-roundcube", :downloads=>3956},
 {:module=>"tohuwabohu-confluence", :downloads=>3790},
 {:module=>"flypenguin-teamcity", :downloads=>3137},
 {:module=>"benforge-activemq", :downloads=>2021},
 {:module=>"benforge-jbosseap", :downloads=>1740},
 {:module=>"GobuNatarajan-gsutil", :downloads=>1510},
 {:module=>"puppetlabs-rkt", :downloads=>1423},
 {:module=>"RanjithKumar45-gcloudsdk", :downloads=>0}]

igalic · 2016-07-07T19:43:31Z

nice! thank you for the stats!

igalic · 2016-07-08T14:30:30Z

okay, so, in my archive::download patch i made some adjustments to fix up incompatibilities… which makes me wonder what i can do to archive itself to discover usage of camptocamp style sematics vs voxpupuli style.

i'm also contemplating to copy camptocamp's spec tests to see how compatible we are — but not sure how that'll actually work, since they are written for a defined type, and ours is a type/provider

our type now accepts the same parameters as camptocamp/archive and processes them correctly.

since camptocamp/archive is apache-2.0 licensed, we can simply copy it! the body is filled with a call archive.

igalic · 2016-07-08T18:20:56Z

someone tried using this patchset as replacement for camptocamp/archive and reported the following error:

Error: Failed to apply catalog: Parameter checksum failed on Archive[/var/lib/jenkins/plugins/workflow-scm-step.hpi]: Invalid value "". Valid values are true, false. Valid values match /\b[0-9a-f]{5,128}\b/. at /etc/puppetlabs/code/environments/dev/modules/archive/manifests/download.pp:54

igalic · 2016-07-08T19:32:01Z

tests green except for that blacksmith/rest-client issue, for which we have a pr lined up here voxpupuli/puppet-blacksmith#35

that default should be false — for maximum compatibility with both modules

igalic · 2016-07-08T20:17:47Z

lib/puppet/type/archive.rb

  newparam(:cookie) do
    desc 'archive file download cookie.'
  end

  newparam(:checksum) do
    desc 'archive file checksum (match checksum_type).'
+    newvalues(%r{\b[0-9a-f]{5,128}\b}, :true, :false, :undef, nil, '')


this is awful

but it works with the latest _release_ or rtyler/jenkins

(a.k.a.: we're running it in prod now ;)

raphink · 2016-07-12T10:02:44Z

alexjfisher · 2016-07-13T15:27:32Z

manifests/download.pp

@@ -0,0 +1,64 @@
+# == Definition: archive::download


@igalic Maybe document this type as for compatibility with camptocamp? You would otherwise always use the native archive type directly?

it wasn't documented in camptocamp/archive either _
i don't want to document it, because it's just a shim. private api that moves as camptocamp/archive moves.

sorry, I wasn't clear (story of my life!) I don't mean document all the parameters etc. I mean just add a line to say that it is a shim/private etc. Something to discourage new users from thinking this is the way they should be using the module.

similar to how the new parameters in the type are marked (this parameter is for camptocamp/archive compatibility)

alexjfisher · 2016-07-13T15:29:52Z

Other than a updating the README and comments in archive::download LGTM. 👍

make our type camptocamp/archive compatible

igalic mentioned this pull request Jun 21, 2016

conflicting with camptocamp/archive #165

Closed

igalic reviewed Jun 21, 2016
View reviewed changes

igalic changed the title ~~first stab at camptocamp/archive compatibility~~ make archive type camptocamp/archive compatible Jun 21, 2016

igalic changed the title ~~make archive type camptocamp/archive compatible~~ make our type camptocamp/archive compatible Jun 21, 2016

igalic mentioned this pull request Jul 8, 2016

(MODULES-3581) Add rest-client to fix ruby 1.9 issues puppetlabs/modulesync_configs#75

Merged

igalic added 2 commits July 8, 2016 20:01

make type camptocamp/archive compatible

fc80536

our type now accepts the same parameters as camptocamp/archive and processes them correctly.

camptocamp/archive compat: add archive::download

7074465

since camptocamp/archive is apache-2.0 licensed, we can simply copy it! the body is filled with a call archive.

ensure that checksum has sensible default for undef

0ef5362

that default should be false — for maximum compatibility with both modules

igalic reviewed Jul 8, 2016
View reviewed changes

igalic mentioned this pull request Jul 13, 2016

Update metadata.json dependencies and requirements #181

Merged

alexjfisher reviewed Jul 13, 2016
View reviewed changes

nibalizer merged commit a5f344e into voxpupuli:master Jul 13, 2016

igalic deleted the camptocamp-compat branch July 13, 2016 18:21

igalic mentioned this pull request Jul 26, 2016

'archive' module conflict voxpupuli/puppet-kafka#61

Closed

alexjfisher mentioned this pull request Aug 14, 2017

allow_insecure is not working #294

Closed

cegeka-jenkins pushed a commit to cegeka/puppet-archive that referenced this pull request Mar 26, 2021

Merge pull request voxpupuli#176 from igalic/camptocamp-compat

dfd2372

make our type camptocamp/archive compatible

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make our type camptocamp/archive compatible #176

make our type camptocamp/archive compatible #176

igalic commented Jun 21, 2016 •

edited

igalic Jun 21, 2016

daenney commented Jun 21, 2016 •

edited

igalic commented Jun 21, 2016

igalic commented Jun 21, 2016

lesaux commented Jun 21, 2016

igalic commented Jun 21, 2016

igalic commented Jul 6, 2016 •

edited

alexjfisher commented Jul 7, 2016

igalic commented Jul 7, 2016 •

edited

jyaworski commented Jul 7, 2016

alexjfisher commented Jul 7, 2016

alexjfisher commented Jul 7, 2016

igalic commented Jul 7, 2016

igalic commented Jul 8, 2016

igalic commented Jul 8, 2016

igalic commented Jul 8, 2016

igalic Jul 8, 2016

igalic Jul 8, 2016

igalic Jul 8, 2016

raphink commented Jul 12, 2016

alexjfisher Jul 13, 2016

igalic Jul 13, 2016

alexjfisher Jul 13, 2016 •

edited

alexjfisher Jul 13, 2016

alexjfisher commented Jul 13, 2016

make our type camptocamp/archive compatible #176

make our type camptocamp/archive compatible #176

Conversation

igalic commented Jun 21, 2016 • edited

igalic Jun 21, 2016

Choose a reason for hiding this comment

daenney commented Jun 21, 2016 • edited

igalic commented Jun 21, 2016

igalic commented Jun 21, 2016

lesaux commented Jun 21, 2016

igalic commented Jun 21, 2016

igalic commented Jul 6, 2016 • edited

alexjfisher commented Jul 7, 2016

igalic commented Jul 7, 2016 • edited

jyaworski commented Jul 7, 2016

alexjfisher commented Jul 7, 2016

alexjfisher commented Jul 7, 2016

igalic commented Jul 7, 2016

igalic commented Jul 8, 2016

igalic commented Jul 8, 2016

igalic commented Jul 8, 2016

igalic Jul 8, 2016

Choose a reason for hiding this comment

igalic Jul 8, 2016

Choose a reason for hiding this comment

igalic Jul 8, 2016

Choose a reason for hiding this comment

raphink commented Jul 12, 2016

alexjfisher Jul 13, 2016

Choose a reason for hiding this comment

igalic Jul 13, 2016

Choose a reason for hiding this comment

alexjfisher Jul 13, 2016 • edited

Choose a reason for hiding this comment

alexjfisher Jul 13, 2016

Choose a reason for hiding this comment

alexjfisher commented Jul 13, 2016

igalic commented Jun 21, 2016 •

edited

daenney commented Jun 21, 2016 •

edited

igalic commented Jul 6, 2016 •

edited

igalic commented Jul 7, 2016 •

edited

alexjfisher Jul 13, 2016 •

edited