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

Move Bundler API to Rubygems.org #966

Closed
wants to merge 0 commits into from
Closed

Move Bundler API to Rubygems.org #966

wants to merge 0 commits into from

Conversation

maclover7
Copy link
Contributor

@maclover7 maclover7 commented May 28, 2015

PR to combine Bundler API's Sinatra app with Rubygems.org's Rails app.

cc @indirect

TODO:

  • Import bundler-api code
  • Rails-ify
  • Remove duplicate tasks?
  • Figure out subdomain/routing stuff --> use the "api/v1/dependencies" endpoint
  • Add caching
  • Provision memcached in production (@dwradcliffe)
  • Make sure tests are passing
  • performance/load testing
  • 🎉

@dwradcliffe
Copy link
Member

Let's get rid of librato before doing this.

@maclover7
Copy link
Contributor Author

@dwradcliffe What metrics service are we moving to instead of Librato?

@segiddins
Copy link
Member

@maclover7 maybe a subtree merge would be better, so we can preserve history?

@indirect
Copy link
Member

@segiddins since I expect this PR to reimplement the API as a rails controller before it's merged, I think the history probably isn't worth it in this particular case. Good idea, though.

On Wed, May 27, 2015 at 9:36 PM, Samuel E. Giddins
notifications@github.com wrote:

@maclover7 maybe a subtree merge would be better, so we can preserve history?

Reply to this email directly or view it on GitHub:
#966 (comment)

@arthurnn
Copy link
Member

Agree with @indirect . I dont think we should port the code honestly.
We should rewrite it in a Rails controller, from scratch, having the new api as base line.
I think old bundler-api server can still run for the old api, and rubygems.org will only have the new API.

@maclover7
Copy link
Contributor Author

What domain/sub domain will we be running the API on? bundler.rubygems.org? We have to keep in mind that v2 (new index) API will be coming out soon...

@bf4
Copy link
Contributor

bf4 commented May 28, 2015

any reason not to do this as an engine/rack app in another repo and mount it?

@indirect
Copy link
Member

@B4F because it would make the rg.org depend on more stacks to run (Sinatra/sequel as well as Rails/AR). This isn't super important, but it's a nice to have as we transition away from the API being the main index that people use.

@maclover7
Copy link
Contributor Author

@indirect @qrush Does the current Bundler API and the Rubygems.org API overlap at all with their functionality? (I'm not very familiar with the codebase)

@indirect
Copy link
Member

@maclover7 nope, no overlap. The Bundler API only provides dependencies, and the current Rubygems API has no equivalent.

@maclover7
Copy link
Contributor Author

Gotcha. Was just wondering if everything needed to be written from scratch, or if there was any starting point. I know @dwradcliffe said we would be moving off of Librato -- what stats logger are we moving to?

@dwradcliffe
Copy link
Member

@maclover7 We're moving everything to datadog/statsd. I think @andremedeiros is working on migrating it.

@maclover7
Copy link
Contributor Author

Got done a lot of work with @indirect, we Rails-ified the web process. Only thing left to do is to integrate Memcached (ping @dwradcliffe).

def index
return render text: "Too many gems!", status: 422 if gem_names.size > GEM_REQUEST_LIMIT

deps = gem_names.each_with_object([]) do |gem_name, deps_array|
Copy link
Member

Choose a reason for hiding this comment

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

Can we get this all in a model? DependencyResolver or something. This is a ton of logic for a controller.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, we would call DependencyResolver.new(gem_names) ?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. And then you can to_json or to_marshal it in the respond_to block.

@qrush
Copy link
Member

qrush commented Jun 26, 2015

Can we port over the bundler-api tests too?

@maclover7
Copy link
Contributor Author

Do we want to port those over from RSpec?

@qrush
Copy link
Member

qrush commented Jun 26, 2015

Without a doubt. It should be pretty straightforward.

@maclover7
Copy link
Contributor Author

@indirect Removed the memcached travis service commit. Rebased on master, and now Github is saying there are merge conflicts (which there aren't -- git locally on my machine says I'm up to date). Travis doesn't create a build job unless Github says there are no merge conflicts

@dwradcliffe
Copy link
Member

@maclover7 merge conflict is probably in the routes

@segiddins
Copy link
Member

@maclover7 any interest in revisiting this?

@maclover7
Copy link
Contributor Author

I'll take a look at this tonight, and bring this up to speed. :)

@maclover7 maclover7 closed this Mar 22, 2016
@maclover7
Copy link
Contributor Author

Uh, what? Rebasing appears to be have closed my PR...?

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.

7 participants