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

Add HTTP method to middleware request object #1386

Merged
merged 6 commits into from
Jan 10, 2018

Conversation

matiasinsaurralde
Copy link
Contributor

This covers both coprocess & JSVM, related to #1219.

@buger
Copy link
Member

buger commented Jan 8, 2018

I see you added the following comment:

There's a data structure that's based on this and it's used for Protocol Buffer support, make sure to update it on every change (coprocess/proto/coprocess_session_state.proto).

But what about coprocess/coprocess_session_state.pb.go, does it gets updated automatically?

If so, what commands I should run to do that?

return beta_implementations.dynamic_stub(channel, 'coprocess.Dispatcher', cardinalities, options=stub_options)
except ImportError:
pass
import grpc
Copy link
Member

Choose a reason for hiding this comment

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

This stuff is new, did you used some experimental protobuf version to build this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Dispatcher class looks mostly the same, theBetaDispatcher changed a bit, I will investigate this (shouldn't affect).

Copy link
Member

Choose a reason for hiding this comment

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

Now it required grpc packages (previously it dont).

I would say that it is better to run script with same protobuf version as we officially use now in CI.

@matiasinsaurralde
Copy link
Contributor Author

I've updated my Protocol Buffers setup to make sure that we're using the latest stable version, so the bindings have been generated using grpcio-1.8.3 (installed via pip) and protobuf-3.5.1.
protoc returns the following:

$ protoc --version
libprotoc 3.5.1

I have also updated the instructions and the script that we use to generate the bindings to explicitly use Python 3 (previously it could mess things up if you ran the Python generator using "python" instead of "python3").
I will push the second PR that extends our definitions in a few minutes but would like to get this merged first so that we can rebase the other PR.

@buger buger merged commit 5984788 into TykTechnologies:master Jan 10, 2018
letzya pushed a commit that referenced this pull request Jan 11, 2018
Both JSVM and Coprocess. 

In addition, fixed missing "certificate" field.
letzya pushed a commit that referenced this pull request Jan 12, 2018
Both JSVM and Coprocess.

In addition, fixed missing "certificate" field.
buger pushed a commit that referenced this pull request Jan 17, 2018
Both JSVM and Coprocess. 

In addition, fixed missing "certificate" field.
excieve pushed a commit to excieve/tyk that referenced this pull request Feb 9, 2018
Both JSVM and Coprocess. 

In addition, fixed missing "certificate" field.
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.

2 participants