Adding Infra and Readme #9

Bento007 · 2020-01-28T20:48:53Z

Adding instruction to create an AWS S3 bucket to store the
Adding requirements-dev.txt
Adding Directory for consumer code
Adding terraform to build the consumer bucket
Adding a terraform framework to further build out our terraform.

- Adding instruction to create an AWS S3 bucket to store the - Adding requirements-dev.txt - Adding Directory for consumer code - Adding terraform to build the consumer bucket - Adding a terraform framework to further build out our terraform.

Bento007 · 2020-01-28T21:12:01Z

I will rename consumer to browser

maniarathi

Please run a linter on these files.. looks like there are a number of formatting issues potentially.

maniarathi · 2020-01-28T21:07:02Z

infra/consumer/main.tf

+  }
+}
+
+#TODO make policy


Just to start good practices, every time we have a TODO, could you please create a ticket in the repo that is tracked and then link the ticket in the TODO itself?

So format would look like:
# TODO(https://github.com/chanzuckerberg/dcp-prototype/pull/9): Make policy.

maniarathi · 2020-01-28T21:07:42Z

infra/consumer/main.tf

@@ -0,0 +1,22 @@
+#make bucket


Why is this commented out? As a good practice, let's try to avoid commented out code.

that's not code. It's just a comment.

maniarathi · 2020-01-28T21:08:21Z

infra/build_deploy_config.py

@@ -0,0 +1,70 @@
+#!/usr/bin/env python
+
+import os


I think ordering of the imports is incorrect. Did you run a linter on this?

maniarathi · 2020-01-28T21:09:47Z

infra/build_deploy_config.py

+        profile_setting=profile_setting,
+    ))
+
+with open(os.path.join(infra_root, args.component, "variables.tf"), "w") as fp:


Please try to use descriptive names instead of a few letters representing the object.
fp -> terraform_file_pointer for example.

I see no problem with this, name since the name is not used outside of this context and the function is short. fp is also commonly used in many languages to mean file pointer.

maniarathi · 2020-01-28T21:13:14Z

infra/build_deploy_config.py

+infra_root = os.path.abspath(os.path.dirname(__file__))
+
+
+parser = argparse.ArgumentParser(description=__doc__)


Please import each module using the full pathname location of the module:

from argparse import ArgumentParser

Reference to style guide: http://google.github.io/styleguide/pyguide.html#23-packages

maniarathi · 2020-01-28T21:14:32Z

dcp-prototype/consumer/README.md

+
+Clone the repo and install dependencies:
+```
+git clona git@github.com:chanzuckerberg/dcp-prototype.git


s/clona/clone

maniarathi · 2020-01-28T21:15:25Z

dcp-prototype/consumer/README.md

@@ -0,0 +1,30 @@
+# Consumer Prototype


As per our offline conversation, please avoid using consumer/contributor and instead use the module name itself.

maniarathi · 2020-01-28T21:16:06Z

dcp-prototype/consumer/README.md

+
+Also install [terraform from Hashicorp](https://www.terraform.io/) from your favourite package manager.
+
+# deploy infra


Keep it neat, please. s/deploy infra/Deploy Infrastructure. Or the like.

maniarathi · 2020-01-28T21:16:45Z

.gitignore

@@ -127,3 +127,7 @@ dmypy.json

 # Pyre type checker
 .pyre/
+backend.tf


Maybe add a separate section for terraform?

# Terraform backend.tf providers.tf variables.tf .terraform

maniarathi · 2020-01-28T21:17:52Z

dcp-prototype/browser/README.md

+pip install -r requirements.txt
+```
+
+Also install [terraform from Hashicorp](https://www.terraform.io/) from your favourite package manager.


favourite? :)

ryanking · 2020-01-28T21:31:03Z

A lot of the stuff you are doing here is already handled (for free) by fogg. Is there a reason you are not using it?

mweiden · 2020-01-28T21:51:09Z

@Bento007 this might be a good chance for you to check in with @ryanking, learn more about core infra's toolchain, and evaluate it for use on DCP 2.0.

mckinsel · 2020-01-28T21:51:39Z

@ryanking perhaps you could point to some examples or sketch out how this could be done with fogg? The fogg docs say "update fogg.json", which is a little "draw the rest of the owl" as far as instructions go.

mckinsel · 2020-01-29T03:29:21Z

Hey @Bento007 , if this is /browser where should Colin be putting his frontend code?

ryanking · 2020-01-29T16:51:43Z

@mckinsel yeah our docs are pretty light. I scheduled time with @Bento007 to give an overview

# This is the 1st commit message: Add high level tissue dimension to integrated corpus # This is the commit message #2: fix bug # This is the commit message #3: Change tissue layout # This is the commit message #4: Add to summary cubes # This is the commit message #5: fix bug # This is the commit message #6: Modify placeholder function # This is the commit message #7: Fix bug # This is the commit message #8: Fix bug # This is the commit message #9: Add extra dimension to validation # This is the commit message #10: Update query, update unitest # This is the commit message #11: Refractor query builder

Adding terraform builder

18d6753

- Adding instruction to create an AWS S3 bucket to store the - Adding requirements-dev.txt - Adding Directory for consumer code - Adding terraform to build the consumer bucket - Adding a terraform framework to further build out our terraform.

Bento007 requested review from ryanking and maniarathi January 28, 2020 20:48

Bento007 self-assigned this Jan 28, 2020

Bento007 requested a review from mckinsel January 28, 2020 20:49

rename consumer to browser

5d0b250

maniarathi suggested changes Jan 28, 2020

View reviewed changes

Bento007 closed this Jan 31, 2020

Bento007 deleted the tsmith/consumer-deploy branch February 20, 2020 23:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Infra and Readme #9

Adding Infra and Readme #9

Bento007 commented Jan 28, 2020

Bento007 commented Jan 28, 2020

maniarathi left a comment

maniarathi Jan 28, 2020

maniarathi Jan 28, 2020

Bento007 Jan 28, 2020

maniarathi Jan 28, 2020

maniarathi Jan 28, 2020

Bento007 Jan 30, 2020

maniarathi Jan 28, 2020

maniarathi Jan 28, 2020

maniarathi Jan 28, 2020

maniarathi Jan 28, 2020

maniarathi Jan 28, 2020

maniarathi Jan 28, 2020

ryanking commented Jan 28, 2020

mweiden commented Jan 28, 2020

mckinsel commented Jan 28, 2020

mckinsel commented Jan 29, 2020

ryanking commented Jan 29, 2020

		infra_root = os.path.abspath(os.path.dirname(__file__))


		parser = argparse.ArgumentParser(description=__doc__)


		Also install [terraform from Hashicorp](https://www.terraform.io/) from your favourite package manager.

		# deploy infra

Adding Infra and Readme #9

Adding Infra and Readme #9

Conversation

Bento007 commented Jan 28, 2020

Bento007 commented Jan 28, 2020

maniarathi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanking commented Jan 28, 2020

mweiden commented Jan 28, 2020

mckinsel commented Jan 28, 2020

mckinsel commented Jan 29, 2020

ryanking commented Jan 29, 2020