Skip to content

Add 2 more values to template vars to make multiple backend support better#694

Merged
qiwzhang merged 6 commits intocloudendpoints:masterfrom
maroux:multi_service_template
Sep 11, 2019
Merged

Add 2 more values to template vars to make multiple backend support better#694
qiwzhang merged 6 commits intocloudendpoints:masterfrom
maroux:multi_service_template

Conversation

@maroux
Copy link
Copy Markdown
Contributor

@maroux maroux commented Sep 10, 2019

This allows us to use a template that can leverage ${services} rather than having to hardcode the list of multiple services when using --experimental_enable_multiple_api_configs.

qiwzhang
qiwzhang previously approved these changes Sep 10, 2019
@maroux maroux force-pushed the multi_service_template branch 2 times, most recently from 18afee2 to a590f71 Compare September 11, 2019 02:50
@maroux maroux force-pushed the multi_service_template branch from a590f71 to 85bc486 Compare September 11, 2019 02:51
@qiwzhang
Copy link
Copy Markdown
Contributor

E2E tested failed with errors:

Traceback (most recent call last):
File "/home/jenkins/.cache/bazel/_bazel_root/752dabe8e6eb24287f1fbb3854c08f93/sandbox/linux-sandbox/69/execroot/main/bazel-out/k8-opt/bin/start_esp/test/start_esp_test.runfiles/main/start_esp/start_esp.py", line 1107, in
write_nginx_conf(ingress, nginx_conf, args)
File "/home/jenkins/.cache/bazel/_bazel_root/752dabe8e6eb24287f1fbb3854c08f93/sandbox/linux-sandbox/69/execroot/main/bazel-out/k8-opt/bin/start_esp/test/start_esp_test.runfiles/main/start_esp/start_esp.py", line 168, in write_nginx_conf
services=args.services)
AttributeError: 'Namespace' object has no attribute 'services'

Comment thread start_esp/start_esp.py Outdated
@maroux
Copy link
Copy Markdown
Contributor Author

maroux commented Sep 11, 2019

E2E tested failed with errors:

Traceback (most recent call last):
File "/home/jenkins/.cache/bazel/_bazel_root/752dabe8e6eb24287f1fbb3854c08f93/sandbox/linux-sandbox/69/execroot/main/bazel-out/k8-opt/bin/start_esp/test/start_esp_test.runfiles/main/start_esp/start_esp.py", line 1107, in
write_nginx_conf(ingress, nginx_conf, args)
File "/home/jenkins/.cache/bazel/_bazel_root/752dabe8e6eb24287f1fbb3854c08f93/sandbox/linux-sandbox/69/execroot/main/bazel-out/k8-opt/bin/start_esp/test/start_esp_test.runfiles/main/start_esp/start_esp.py", line 168, in write_nginx_conf
services=args.services)
AttributeError: 'Namespace' object has no attribute 'services'

Yep, tests weren't working for my local Mac, so took a few commits to fix 😄

Comment thread start_esp/start_esp.py
logging.error("[ESP] Service name is not specified");
sys.exit(3)

args.services = args.service.split('|')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think args.services should already be assigned. This line is not needed here, I think

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is needed because if args.service is not set, and it's fetched from the metadata server, then it's only set in line 361. I suppose this could be moved closer to that to make the association clearer.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please don't do too many code refactory. I don't want to break most of users that only using one service

@qiwzhang qiwzhang merged commit f9e2b7e into cloudendpoints:master Sep 11, 2019
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