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

Initial support for App Engine deployment provider #221

Closed
wants to merge 13 commits into from

Conversation

imjasonh
Copy link

@imjasonh imjasonh commented Feb 5, 2015

This change adds appengine.rb which shells out to gcloud to deploy to App Engine.

@imjasonh
Copy link
Author

imjasonh commented Feb 5, 2015

Looks like I've got a bug, installing gcloud prompts for confirmation about where to install, which hangs forever (https://travis-ci.org/travis-ci/dpl/builds/49640908) I'll update this PR when I've gotten past there.

@imjasonh
Copy link
Author

imjasonh commented Feb 6, 2015

@BanzaiMan do you want to take a look at this as well? It's basically the same as gae.rb except it uses gcloud instead of appcfg, which will give it more flexibility/features in future releases.

@BanzaiMan BanzaiMan self-assigned this Feb 8, 2015
@hello-josh
Copy link

@imjasonh I am new to travis-ci and know nothing about ruby but I do know a fair amount about appengine. Would it be difficult to allow the user to pass the project id and/or version by either .travis.yml defined or environment variables? I am using travis to deploy branches to different version labels so being able to use some form of $TRAVIS_BRANCH and passing it to gcloud would be nice

@imjasonh
Copy link
Author

@TRII That's an interesting idea. I'd kind of like to get the base functionality there first then add features on top, but that's definitely something to consider.

It might be easier to discuss your proposal if you described it in terms of what the .travis.yml would specify and what gcloud would run as a result. Deploying branches to different versions, or even each commit to a different version could be nice additions though.

Could you email me off this bug since it's sort of off-topic for this particular minimal change?

@meatballhat
Copy link
Contributor

@imjasonh Thanks so much for the contribution! A few thoughts:

  • I find the relationship between gae and appengine confusing. Can they be combined into one provider with two strategies, or can we perhaps consider scrapping appcfg usage altogether?
  • The gae and appengine providers are the only ones using the open-uri library. I would prefer to see wget being used as with other providers.

@imjasonh
Copy link
Author

I find the relationship between gae and appengine confusing. Can they be combined into one provider with two strategies, or can we perhaps consider scrapping appcfg usage altogether?

Agreed, I think we'd prefer to have only one provider, and I think we'd prefer to use gcloud instead of appcfg, since gcloud is more actively developed and will hopefully eventually replace appcfg completely.

The only reason I didn't make this PR just alter the behavior of the gae provider is that I didn't want to risk breaking any users of that provider, but it's experimental so I guess that's not too bad. I also don't know whether we prefer gae or appengine.

The gae and appengine providers are the only ones using the open-uri library. I would prefer to see wget being used as with other providers.

Yes, I can make that change for the appengine provider, and gae as well if you'd prefer.

@imjasonh
Copy link
Author

imjasonh commented Mar 4, 2015

Re: the appengine vs gae point -- I would be fine replacing the appcfg usage with gcloud entirely, in the gae provider. I believe I would just need to add the ability to specify an app_dir which the gae provider currently does.

@BanzaiMan you seem to be the most knowledgeable about the gae provider -- do you think that we could replace its usage of appcfg with gcloud without breaking users? Does being tagged as experimental mean that any such breakage would be acceptable?

@BanzaiMan
Copy link
Contributor

@imjasonh Sorry for the delay.

What, if any, user facing breaking changes are there if we move to gcloud?

@BanzaiMan
Copy link
Contributor

@imjasonh Just pinging you to keep this on your radar.

@imjasonh
Copy link
Author

I think we've decided to put this on the back burner while we sort out what the surface for gcloud app will be. It's currently a preview feature, which means it can change in breaking ways, and I'd like to avoid having to chase those changes with this plugin and cause unnecessary work and headaches.

When gcloud app settles, I'll resurrect this PR and switch over to use that instead of appcfg

Sorry for the confusion!

@imjasonh imjasonh closed this Jul 17, 2015
@BanzaiMan
Copy link
Contributor

@imjasonh No worries. Ping us again when you're ready! :-D

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.

4 participants