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

Script for setting up Apigateway base path maping #1247

Merged
merged 2 commits into from
May 30, 2018

Conversation

xbrianh
Copy link
Member

@xbrianh xbrianh commented May 18, 2018

A script for idempotently mapping the AWS Apigateway custom domain name to the Apigateway stage. It should be executed for a first-time deployment after successfully running make deploy-infra and make deploy.

#connect to #856

A script for idempotently mapping the AWS Apigateway custom domain
name to the Apigateway stage. It should be executed for a
first-time deployment after successfully running `make deploy-infra`
and `make deploy`.
@xbrianh xbrianh self-assigned this May 18, 2018
@xbrianh xbrianh requested review from kislyuk and hannes-ucsc May 18, 2018 22:00
Copy link
Contributor

@hannes-ucsc hannes-ucsc left a comment

Choose a reason for hiding this comment

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

I'm starting to worry that we have no tests for this code. I'm aware that it is a helper script, nevertheless I'd feel better if we had a regularly running test that exercised this functionality. Do you have any plans in this respect?

#!/usr/bin/env python

"""
This script idempotently maps the AWS Apigateway custom domain name to the Apigateway
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Apigateway is a really odd spelling. Amazon API Gateway is the official spelling.

stage = os.environ['DSS_DEPLOYMENT_STAGE']
domain_name = os.environ['API_DOMAIN_NAME']

APIGATEWAY = boto3.client('apigateway')
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: The convention of upper-casing variables referencing boto3 clients objects has no precedence in this project. I would suggest sticking with the existing convention or filing a PR that establishes this convention consistently across the project after motivating it to the team.

Copy link
Contributor

@hannes-ucsc hannes-ucsc May 22, 2018

Choose a reason for hiding this comment

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

@Bento007 started upper-casing non-constants so we now have zero an annoying degree of inconsistency.

@xbrianh xbrianh merged commit 387d908 into master May 30, 2018
@xbrianh xbrianh deleted the bhannafi-infra-base-path-mapping branch May 30, 2018 18:02
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