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 endpoints api_manager codes #1

Merged
merged 3 commits into from
Dec 2, 2016
Merged

Conversation

qiwzhang
Copy link
Contributor

@qiwzhang qiwzhang commented Dec 2, 2016

No description provided.

@qiwzhang
Copy link
Contributor Author

qiwzhang commented Dec 2, 2016

@kyessenov, @wlu2016, could you help to review this?

@@ -0,0 +1 @@
1.1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is version here and not in ESP?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are two places need version: service_control/proto.cc and cloud_trace.
They like to send service agent with version.

@kyessenov
Copy link
Contributor

It's very difficult to navigate a PR of this size. Here are some other observations:

  • WORKSPACE should be in the top level per Bazel expectations; also makes sense to move third_party to the top level, too.

  • why didn't you flatten api_manager directory out? make it contrib/endpoints/config.cc, for example.

  • maybe we should move server config out of here, too?

@qiwzhang
Copy link
Contributor Author

qiwzhang commented Dec 2, 2016

  1. For now, it is easier to put WORKSPACE in contrib/endpoints. Other folder may not use bazel.
    If they use bazel, a repo can have multiple WORKSPACE too
    Or we could do the move in later phase. Don't want to introduce too big change in one PR.

  2. we are in contrib/endpoints. In the future, it may have other endpoint components beside api_manager.

  3. before we have alternative to replace server_config, we have to keep it here.

Copy link
Contributor

@smawson smawson left a comment

Choose a reason for hiding this comment

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

Just marking approved for initial commit :)

@smawson smawson merged commit 11472b5 into istio:master Dec 2, 2016
@qiwzhang qiwzhang deleted the init branch December 2, 2016 03:56
qiwzhang added a commit that referenced this pull request Feb 16, 2018
bianpengyuan added a commit to bianpengyuan/proxy that referenced this pull request Jul 16, 2019
brian-avery pushed a commit that referenced this pull request Oct 8, 2019
adopt istio/envoy fix for CVE-2019-15226

Signed-off-by: Yuchen Dai <[email protected]>
YaoZengzeng pushed a commit to YaoZengzeng/proxy that referenced this pull request Jun 24, 2024
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.

3 participants