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 #22141 - Remove sinatra/rack dependencies #42

Merged
merged 1 commit into from Jan 23, 2018

Conversation

dLobatog
Copy link
Member

@dLobatog dLobatog commented Jan 4, 2018

Without this, I'm getting the following error when running bundle update on smart-proxy (develop branch). I suppose the plugin can simply rely on sinatra/rack being provided by smart-proxy.

Resolving dependencies......
Bundler could not find compatible versions for gem "rack-protection":
  In Gemfile:
    smart_proxy was resolved to 1.17.0.develop, which depends on
      sinatra was resolved to 1.4.6, which depends on
        rack-protection (= 2.0.0)

    smart_proxy_dynflow_core was resolved to 0.1.8, which depends on
      sinatra (~> 1.4) was resolved to 1.4.6, which depends on
        rack-protection (~> 1.4)
Bundler could not find compatible versions for gem "tilt":
  In Gemfile:
    smart_proxy was resolved to 1.17.0.develop, which depends on
      sinatra was resolved to 1.4.0, which depends on
        tilt (~> 2.0)

    smart_proxy_dynflow_core was resolved to 0.1.8, which depends on
      sinatra (~> 1.4) was resolved to 1.4.0, which depends on
        tilt (>= 1.3.4, ~> 1.3)

@theforeman-bot
Copy link
Member

There were the following issues with the commit message:

  • 02a245a must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.


This message was auto-generated by Foreman's prprocessor

@theforeman-bot
Copy link
Member

@dLobatog, the Redmine ticket used is for a different project than the one associated with this GitHub repository. Please either:

If changing the ticket number used, remember to update the PR title and the commit message (using git commit --amend).


This message was auto-generated by Foreman's prprocessor

@dLobatog dLobatog changed the title Remove sinatra/rack dependencies Fixes #22141 - Remove sinatra/rack dependencies Jan 4, 2018
@iNecas
Copy link
Member

iNecas commented Jan 4, 2018

We can't, as this is for smart_proxy_dynflow_core, which can run separately but needs sinatta for dynflow console and internal api

@dLobatog
Copy link
Member Author

dLobatog commented Jan 4, 2018

@iNecas How do you run it then with smart-proxy nightly?

Copy link
Member

@iNecas iNecas left a comment

Choose a reason for hiding this comment

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

I was not able to reproduce this issue. I guess the issue might be that you had installed proxy without the plugin and then added it later? I tried to do so as well, but bundler (I tested with 1.15.4 and 1.16.1) was able to find the proper versions.

I've tested this with removing the explicit requirement on sinatra version and it was working for me, therefor I suggest for removing the ~> 1.4.0, which I believe should solve your issues as well.

@adamruzicka
Copy link
Contributor

I was able to reproduce the issue @dLobatog describes when running dynflow core as part of smart proxy.

@iNecas
Copy link
Member

iNecas commented Jan 23, 2018

@dLobatog ping

Copy link
Member Author

@dLobatog dLobatog left a comment

Choose a reason for hiding this comment

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

@iNecas pong

@@ -34,7 +34,7 @@ Gem::Specification.new do |gem|
gem.add_runtime_dependency('foreman-tasks-core', '>= 0.1.7')
gem.add_runtime_dependency('sequel')
gem.add_runtime_dependency('sqlite3')
gem.add_runtime_dependency('sinatra', '~> 1.4')
gem.add_runtime_dependency('sinatra')
Copy link
Member

Choose a reason for hiding this comment

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

Should this be >= 1.4 instead?

Copy link
Member

Choose a reason for hiding this comment

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

I think we should be ok with whatever we get from the proxy, so I'm ok with this way

Copy link
Member

Choose a reason for hiding this comment

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

The core part doesn't have to run with the proxy I think but I agree that chance is remote.

@iNecas iNecas merged commit 87e0a26 into theforeman:master Jan 23, 2018
@iNecas
Copy link
Member

iNecas commented Jan 23, 2018

Thanks @dLobatog

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