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

Optionally serve housing counselor data from S3 #3107

Merged
merged 3 commits into from Jul 12, 2017

Conversation

chosak
Copy link
Member

@chosak chosak commented Jul 6, 2017

This commit introdces a new HOUSING_COUNSELOR_S3 feature flag that unlocks serving of HUD housing counselor JSON and PDF from a fixed location on S3, instead of the default behavior of dynamically generating on the fly.

This feature flag is configured to look in the URL for a query string parameter s3=True, e.g. visiting the URL

http://localhost:8000/find-a-housing-counselor/?zipcode=18020&s3=True

This will query JSON housing counselor data used to fill out the template from files like

https://s3.amazonaws.com/files.consumerfinance.gov/a/assets/hud/jsons/18020.json

(this uses the AWS_STORAGE_BUCKET_NAME Django setting).

The PDF download link then also changes to just download directly from a prerendered file on S3 like

https://s3.amazonaws.com/files.consumerfinance.gov/a/assets/hud/pdfs/18020.pdf

The pre-existing JSON data format differs slightly from that generated by the django-hud project, mainly by returning lists of languages/services as JSON lists as opposed to comma-delimited strings. For this reason, this commit also includes changes to the existing housing_counselor.html and housing_counselor_pdf.html files to assume a more standard JSON format.

Additions

  • New HOUSING_COUNSELOR_S3 feature flag enabled with a query string ?s3=True that enables using S3 for housing counselor JSON and PDFs.

Changes

  • Various changes to support use of optional feature flag.

Testing

  1. To see default behavior (requires some complex setup to get PDFReactor working locally, so you may just see an error when trying to download a PDF), visit http://localhost:8000/find-a-housing-counselor/ and try the following:
  • Submit the form without entering a zipcode.
  • Submit the form entering an invalid zipcode (e.g. 00000) - note the known bug that puts the map at Usa, Indonesia.
  • Submit the form using a valid zipcode (e.g. 20001) - note the valid results. Clicking "Save list as PDF" will result in an error if PDFReactor is not setup locally.
  1. To see new behavior using the query string-based feature flag, visit http://localhost:8000/find-a-housing-counselor/?s3=True. Note that this requires that your local environment have AWS_STORAGE_BUCKET_NAME=files.consumerfinance.gov.
  • Note that submitting an invalid zipcode now correctly gives an error message instead of putting the map in a random spot.
  • Note that the PDF download link now links directly to S3.

Screenshots

Slight visual change: the PDF download link now (correctly?) includes the PDF icon as is consistent elsewhere on the site.

image

Notes

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows the standards laid out in the development playbook
  • Passes all existing automated tests
  • New functions include new tests
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged
  • Visually tested in supported browsers and devices
  • Project documentation has been updated
  • Reviewers requested with the Assignee tool ➡️

This commit introdces a new HOUSING_COUNSELOR_S3 feature flag that
unlocks serving of HUD housing counselor JSON and PDF from a fixed
location on S3, instead of the default behavior of dynamically
generating on the fly.

This feature flag is configured to look in the URL for a query string
parameter `s3=True`, e.g. visiting the URL

http://localhost:8000/find-a-housing-counselor/?zipcode=18020&s3=True

This will query JSON housing counselor data used to fill out the
template from files like

https://s3.amazonaws.com/files.consumerfinance.gov/a/assets/hud/jsons/18020.json

(this uses the `AWS_STORAGE_BUCKET_NAME` Django setting).

The PDF download link then also changes to just download directly from a
prerendered file on S3 like

https://s3.amazonaws.com/files.consumerfinance.gov/a/assets/hud/pdfs/18020.pdf

The pre-existing JSON data format differs slightly from that generated
by the django-hud project, mainly by returning lists of
languages/services as JSON lists as opposed to comma-delimited strings.
For this reason, this commit also includes changes to the existing
`housing_counselor.html` and `housing_counselor_pdf.html` files to
assume a more standard JSON format.
@richaagarwal
Copy link
Contributor

The code looks good but I'm wondering if this is just an interim solution? It seems like long-term we wouldn't really want to keep that as a param in (people could easily remove it if they wanted, and then we'd have to support code for two types of PDF generation). Is there a reason that the feature flag can't simply enable the s3 functionality, without appending the param to the URL?

@chosak
Copy link
Member Author

chosak commented Jul 12, 2017

@richaagarwal thanks for the review. Yes, the only reason I did it this way is to make it easier for review wherever this gets deployed to, and using a query string seemed easier than, say, a DB-based feature flag which would be more complicated to update/test. Using a query string would let this code all get merged and deployed without having to wait on final decision about switching over.

@chosak chosak merged commit 5c479c9 into master Jul 12, 2017
@chosak chosak deleted the serve-housing-counselors-from-s3 branch July 12, 2017 19:01
chosak added a commit that referenced this pull request Jul 20, 2017
As of #3107, housing counselor JSON and PDF data will be served on the
website from S3. Currently this is behind a feature flag but will be
made default soon.

This change requires knowing which S3 bucket the data lives on, which
must be defined using the `AWS_STORAGE_BUCKET_NAME` environment
variable. This change adds that variable (with value
`files.consumerfinance.gov`) to the sample `.env_SAMPLE` file.
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.

None yet

3 participants