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

Create infrastructure for sample code #331

Merged
merged 3 commits into from Aug 7, 2020
Merged

Create infrastructure for sample code #331

merged 3 commits into from Aug 7, 2020

Conversation

shaunagm
Copy link
Collaborator

@shaunagm shaunagm commented Aug 6, 2020

This is my first stab at creating a new infrastructure for handling sample code. I expect it to need significant edits before merging.

This PR:

  • adds a README describing our sample script process, and including a table listing existing scripts
  • creates a template_script.py file that outlines how we'd like the scripts to be structure
  • edits the existing four scripts to be in line with the template

Some particular feedback I'd like:

  • do the label names I've chosen work? once I get the thumbs up, I'll create the labels and update the PR so the label links in the README link to the labels.
  • does the structure of the template_script look good?
  • does how we're now handling config variables look good? I want to isolate config variables in their own section at the top so they're easy to use. Currently these scripts may overwrite environmental variables, which may not be ideal.
  • have I correctly described what each of the four sample scripts does?

Also if anyone can actually test the refactored sample scripts that'd be swell.

@eliotst eliotst requested review from eliotst and ydamit August 6, 2020 15:45
Copy link
Contributor

@eliotst eliotst left a comment

Choose a reason for hiding this comment

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

I just had a couple of small comments, and some thoughts for the future as we build the scripts out.

useful_resources/sample_code/README.md Outdated Show resolved Hide resolved
useful_resources/sample_code/README.md Outdated Show resolved Hide resolved
useful_resources/sample_code/template_script.py Outdated Show resolved Hide resolved
| ----------- | ----------- | ----------- |
| apply_activist_code.py | Gets activist codes stored in Redshift and applies to users in Van | Redshift, VAN|
| s3_to_redshift.py | Moves files from S3 to Redshift| Redshift, S3|
| s3_to_s3.py | Get files from vendor s3 bucket and moves to own S3 bucket | S3 |
Copy link
Contributor

Choose a reason for hiding this comment

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

I think "vendor" to "own" is maybe what the original script was for, but I think the script can be used for transferring data between any two buckets. I think we can generalize this to just "source" bucket to "target" bucket or something.

"AWS_ACCESS_KEY_ID": "",
"AWS_SECRET_ACCESS_KEY": "",
# S3 (vendor) (note: this assumes a Civis Platform parameter called "AWS_VENDOR")
'AWS_VENDOR_SECRET_ACCESS_KEY': "",
Copy link
Contributor

Choose a reason for hiding this comment

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

If we repurpose as "source" / "destination", we would need to change these environment variable keys.. AWS_SOURCE_SECRET_ACCESS_KEY, AWS_DESTINATION_SECRET_ACCESS_KEY, etc.

@eliotst eliotst merged commit 2c037e3 into move-coop:master Aug 7, 2020
@eliotst eliotst mentioned this pull request Aug 11, 2020
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

2 participants