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

Fixes #8901 - Make new apipie method optional #120

Merged
merged 1 commit into from
Jan 19, 2015

Conversation

GregSutcliffe
Copy link
Member

This is pretty ugly, but it allows us to retain the previous behaviour from before f1f29ca#diff-9c896d7540d8746964738086b33782b7L112 was added (which breaks 1.7 support, and I'd like to keep 1.7 for discovery 2.0). Suggestions on cleaner handling are welcome.

Apipie.configuration.checksum_path += ['/discovered_hosts/']
end
else
if Apipie.configuration.respond_to?(:checksum_path)

Choose a reason for hiding this comment

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

Can stay outside the version conditional?

Copy link
Member

Choose a reason for hiding this comment

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

Whatever, please update so I can test this and give it go. Thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @domcleal, good catch.

@shlomizadok
Copy link
Member

@GregSutcliffe - tested with foreman-1.7-stable. Works well.

@lzap
Copy link
Member

lzap commented Jan 15, 2015

Ha, does not work with develop branch :-(

/home/lzap/.rbenv/versions/2.0.0-p247/lib/ruby/2.0.0/rubygems/version.rb:191:in `initialize': Malformed version number string 1.8.0-develop (ArgumentError)
        from /home/lzap/work/foreman_discovery/lib/foreman_discovery/engine.rb:111:in `new'
        from /home/lzap/work/foreman_discovery/lib/foreman_discovery/engine.rb:111:in `block (2 levels) in <class:Engine>'
        from /home/lzap/work/foreman/app/services/foreman/plugin.rb:64:in `instance_eval'
        from /home/lzap/work/foreman/app/services/foreman/plugin.rb:64:in `register'

@GregSutcliffe
Copy link
Member Author

@lzap ouch, sorry. Updated to work with both 1.7 and nightly. Still ugly though :)

@@ -107,7 +107,10 @@ class Engine < ::Rails::Engine
allowed_template_helpers :rand, :facts_hash

# apipie API documentation
apipie_documented_controllers ["#{ForemanDiscovery::Engine.root}/app/controllers/api/v2/*.rb"]
# Only available in 1.8, otherwise it has to be in the initializer below
if (SETTINGS[:version].to_s.include?('develop') or Gem::Version.new(SETTINGS[:version]) >= Gem::Version.new('1.8'))

Choose a reason for hiding this comment

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

usually we use Gem::Version.new(SETTINGS[:version].chomp('-develop'))

(although ideally we never do version comparisons!)

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed its ugly. @lzap is it better to revert the original change to the apipie usage until after we're done with 1.7?

Copy link
Member

Choose a reason for hiding this comment

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

I am fine with this. Filed issue ticket for 2.1 to remove this workaround: http://projects.theforeman.org/issues/9017

Going to test this. Thanks.

@lzap lzap merged commit 451c486 into theforeman:develop Jan 19, 2015
@lzap
Copy link
Member

lzap commented Jan 19, 2015

Works both for 1.7 and develop. Merged as 451c486 thanks.

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.

5 participants