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

return result in deployProxy including not deployed result #92

Closed
wants to merge 3 commits into from

Conversation

antxxxx
Copy link

@antxxxx antxxxx commented Oct 17, 2017

This change returns deployment information when deployProxy is called so caller can work out which revision was uploaded, where it was deployed to etc.

It also includes a 'not deployed' information when proxy is uploaded but not deployed

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@antxxxx
Copy link
Author

antxxxx commented Oct 17, 2017

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@noahdietz
Copy link
Collaborator

Hi @antxxxx, first of all thanks for the PR. I am on the email thread we have going WRT Hosted Functions, so I am aware of generally why you are submitting this PR.

I'm trying to understand the use case better.

Currently, the deployProxy will return nothing if there was nothing deployed or some deployment info if that proxy was actually deployed to an environment (or multiple).

If I understand, you would like to see deployProxy always return some bit of proxy info regardless of the deployment state of said proxy? I'm curious of when this might happen without the caller knowing...i.e. when using import-only or not. Let me know if I'm misunderstanding!

@antxxxx
Copy link
Author

antxxxx commented Oct 18, 2017

There is a bug in Apigee which means you can not upload service callout policies with loadbalancer elements (we have had an open case with Apigee for several years over this - it fails when you try uploading using curl command so it is not a bug with this program).

We have a couple of proxies with service callout polices with load balancer element, so our CI script replaces this and then uses apigeetool with import only flag and prints a message at the end saying 'revision xxx of proxy yyy needs service callout policy changes and manually deploying'

So we can print this, we need to know the revision that has been uploaded

@noahdietz
Copy link
Collaborator

noahdietz commented Oct 18, 2017 via email

@noahdietz
Copy link
Collaborator

@antxxxx I opened #94 to fix #93

If you are OK with accepting my change instead of yours here, please close this PR. Otherwise, I am happy to continue discussing this!

@antxxxx
Copy link
Author

antxxxx commented Oct 19, 2017

The code in your change is better than mine, so I am happy to accept your change :)

@antxxxx antxxxx closed this Oct 19, 2017
@noahdietz
Copy link
Collaborator

Fixed by #94

Thanks @antxxxx for opening this in the first place!

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.

None yet

4 participants