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

Introduce HTTP 1.1/JSON support for GAPIC clients #857

Merged
merged 21 commits into from
Jan 22, 2018
Merged

Conversation

dwsupplee
Copy link
Contributor

@dwsupplee dwsupplee commented Jan 20, 2018

Toolkit PR: googleapis/gapic-generator#1787 (review)
GAX PR: googleapis/gax-php#118

Todos:

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 20, 2018
@jdpedrie
Copy link
Contributor

this is exciting!!

@michaelbausor
Copy link
Contributor

One extra step we need to do before we release: we need to refresh all of the generated protos in proto-client-php. Otherwise, we might have refreshed gapic clients, but old protos. I will go through and do this now.

@@ -1,12 +1,12 @@
<?php
/*
* Copyright 2017, Google LLC All rights reserved.
* Copyright 2018 Google LLC

This comment was marked as spam.

@@ -1,12 +1,12 @@
<?php
/*
* Copyright 2017, Google LLC All rights reserved.
* Copyright 2018 Google LLC

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@@ -1,12 +1,12 @@
<?php
/*
* Copyright 2017, Google LLC All rights reserved.
* Copyright 2018 Google LLC

This comment was marked as spam.

@@ -1,12 +1,12 @@
<?php
/*
* Copyright 2017, Google LLC All rights reserved.
* Copyright 2018 Google LLC

This comment was marked as spam.

Grpc\STATUS_INTERNAL,
Grpc\STATUS_UNAVAILABLE,
Grpc\STATUS_DATA_LOSS
Code::UNKNOWN,

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@@ -1,12 +1,12 @@
<?php
/*
* Copyright 2017, Google LLC All rights reserved.
* Copyright 2018 Google LLC

This comment was marked as spam.

@@ -1,12 +1,12 @@
<?php
/*
* Copyright 2017, Google LLC All rights reserved.
* Copyright 2018 Google LLC

This comment was marked as spam.

@@ -1,12 +1,12 @@
<?php
/*
* Copyright 2017, Google LLC All rights reserved.
* Copyright 2018 Google LLC

This comment was marked as spam.

*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
* https://www.apache.org/licenses/LICENSE-2.0

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

composer.json Outdated
"guzzlehttp/guzzle": "^5.3|^6.0",
"guzzlehttp/psr7": "^1.2",
"monolog/monolog": "~1",
"psr/http-message": "1.0.*",
"ramsey/uuid": "~3",
"google/proto-client": "^0.30.0",
"google/gax": "^0.29"
"google/gax": "dev-json-transport"

This comment was marked as spam.

This comment was marked as spam.

Copy link
Contributor

@chingor13 chingor13 left a comment

Choose a reason for hiding this comment

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

On the Debugger side, everything looks good here.

@tmatsuo
Copy link
Contributor

tmatsuo commented Jan 22, 2018

Is the system test passing?

@dwsupplee
Copy link
Contributor Author

Running them now, will report back!

@michaelbausor
Copy link
Contributor

proto-client-php 0.31.0 has been released with updated protos (touching Bigtable, BigtableAdmin, Monitoring and OsLogin)

@dwsupplee
Copy link
Contributor Author

Fantastic! Thanks Mike.

private static function getProjectNameTemplate()
{
if (self::$projectNameTemplate == null) {
if (null == self::$projectNameTemplate) {

This comment was marked as spam.

This comment was marked as spam.

Copy link
Contributor

@tmatsuo tmatsuo left a comment

Choose a reason for hiding this comment

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

LGTM

Can you also make sure the system test is passing?

@@ -1,12 +1,12 @@
<?php
/*
* Copyright 2017, Google LLC All rights reserved.
* Copyright 2018 Google LLC

This comment was marked as spam.

@dwsupplee
Copy link
Contributor Author

@tmatsuo System tests came back good.

@dwsupplee
Copy link
Contributor Author

Thanks for the review all!

@dwsupplee dwsupplee merged commit 2f18a3a into master Jan 22, 2018
@dwsupplee dwsupplee mentioned this pull request Jan 22, 2018
@dwsupplee dwsupplee deleted the transport branch January 23, 2018 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants