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

Rewrite artifactory_sha1 function with puppet v4 api #323

Merged

Conversation

alexjfisher
Copy link
Member

Old v3 API function is removed, and new archive::artifactory_checksum
function added. Puppet 4 API functions should not have issues with
environment isolation.

This is a breaking change. The function is renamed and has been
upgraded. It can now return md5 or sha256 checksums as well as sha1.

The function now expects the artifact URL and will convert to the API
url internally. (This was previously done in the calling puppet code.)

@alexjfisher
Copy link
Member Author

@eputnam puppet-strings doesn't seem very happy with me.

puppet strings generate --format markdown
--- SNIP ---
[warn]: Missing documentation for Puppet function 'archive::artifactory_checksum' at lib/puppet/functions/archive/artifactory_checksum.rb:4.
[error]: Unhandled exception in PuppetStrings::Yard::Handlers::Ruby::FunctionHandler:
  in `lib/puppet/functions/archive/artifactory_checksum.rb`:4:

        4: Puppet::Functions.create_function(:'archive::artifactory_checksum') do

[error]: NoMethodError: undefined method `types=' for nil:NilClass
[error]: Stack trace:
        /home/alex/.rvm/gems/ruby-2.4.1/gems/puppet-strings-1.2.1/lib/puppet-strings/yard/handlers/ruby/function_handler.rb:142:in `block in add_overload_tag'
        /home/alex/.rvm/gems/ruby-2.4.1/gems/puppet-strings-1.2.1/lib/puppet-strings/yard/handlers/ruby/function_handler.rb:135:in `each'
        /home/alex/.rvm/gems/ruby-2.4.1/gems/puppet-strings-1.2.1/lib/puppet-strings/yard/handlers/ruby/function_handler.rb:135:in `add_overload_tag'
        /home/alex/.rvm/gems/ruby-2.4.1/gems/puppet-strings-1.2.1/lib/puppet-strings/yard/handlers/ruby/function_handler.rb:100:in `block in add_tags'
        /home/alex/.rvm/gems/ruby-2.4.1/gems/puppet-strings-1.2.1/lib/puppet-strings/yard/handlers/ruby/function_handler.rb:98:in `each'
        /home/alex/.rvm/gems/ruby-2.4.1/gems/puppet-strings-1.2.1/lib/puppet-strings/yard/handlers/ruby/function_handler.rb:98:in `add_tags'

@ekohl
Copy link
Member

ekohl commented Mar 12, 2018

@alexjfisher should the docs be above the function instead of inside it perhaps? Now it looks like it's documenting the dispatch.

@eputnam
Copy link
Member

eputnam commented Mar 12, 2018

having them above the dispatch is correct. i'm going to take a closer look.

@alexjfisher
Copy link
Member Author

I think it's the return_type it doesn't like.

@eputnam
Copy link
Member

eputnam commented Mar 12, 2018

looks like this is another bug. if return_type is used, strings assumes a @return tag is present. if the return tag is not present (or nil), it'll throw this error. if you just add an empty return tag, you can get around this.

dispatch :artifactory_checksum do
param 'Stdlib::HTTPUrl', :url
optional_param "Enum['sha1','sha256','md5']", :checksum_type
return_type 'String'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remove this line, the docs generate.
Equally, I can also add a # @return [String] The checksum above.
I don't think this should be required though as it doesn't add much if I'm already doing the return_type validation thing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old v3 API function is removed, and new `archive::artifactory_checksum`
function added.  Puppet 4 API functions should not have issues with
environment isolation.

This is a breaking change.  The function is renamed and has been
upgraded. It can now return md5 or sha256 checksums as well as sha1.

The function now expects the artifact URL and will convert to the API
url internally. (This was previously done in the calling puppet code.)
@alexjfisher alexjfisher force-pushed the migrate_artifactory_sha1_function branch from fc97bf7 to e872b14 Compare March 13, 2018 12:49
@alexjfisher alexjfisher changed the title WIP: Rewrite artifactory_sha1 function with puppet v4 api Rewrite artifactory_sha1 function with puppet v4 api Mar 13, 2018
@alexjfisher alexjfisher requested a review from ekohl March 13, 2018 13:00
end
# Mock Puppet V4 API ruby function with a puppet language function equivalent
let(:pre_condition) do
'function archive::artifactory_checksum($url,$type) { return \'0d4f4b4b039c10917cfc49f6f6be71e4\' }'
Copy link
Member Author

@alexjfisher alexjfisher Mar 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks to @TraGicCode on slack for suggesting this method of mocking puppet v4 API ruby functions with puppet language functions.

@alexjfisher alexjfisher merged commit e07f463 into voxpupuli:master Mar 16, 2018
@alexjfisher alexjfisher deleted the migrate_artifactory_sha1_function branch March 16, 2018 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants