Skip to content
This repository has been archived by the owner on Jul 3, 2023. It is now read-only.

Adding container image for running examples #209

Merged
merged 4 commits into from
Oct 14, 2022

Conversation

bovem
Copy link
Contributor

@bovem bovem commented Oct 13, 2022

Added dockerfile for testing examples in a container environment.

Changes

  • Added a dockerfile for the image that could be used for running examples.
  • Each example folder has a python virtual environment named hamilton that should be activated first before running the example.

Testing

  • Build the container image
docker build --tag hamilton-example .
  • Creating a container
docker run -it --rm --name hamilton-example hamilton-example
  • Running the hello_world example inside the container
cd hamilton/example/hello_world
source hamilton-env/bin/activate
python my_script.py

Notes

Checklist

  • PR has an informative and human-readable title (this will be pulled into the release notes)
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future TODOs are captured in comments
  • Project documentation has been updated if adding/changing functionality.
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist

Python - local testing

  • python 3.6
  • python 3.7
  • python 3.9 (Docker Environment)

Copy link
Collaborator

@skrawcz skrawcz left a comment

Choose a reason for hiding this comment

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

Just two minor comments, otherwise I ran things and everything worked as specified! Once you have addressed those, this is good to merge. Thanks!

Docker build takes around `6m16.298s` depending on the system configuration and network.
Alternatively, you can pull the container image from here: ...

3. Creating a container
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
3. Creating a container
3. Starting your docker container

```bash
cd hamilton/examples/hello_world
source hamilton-env/bin/activate
python my_script.py
Copy link
Collaborator

@skrawcz skrawcz Oct 14, 2022

Choose a reason for hiding this comment

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

Suggested change
python my_script.py
python my_script.py
deactivate # this will deactivate the python virtual environment, allow you to activate other ones for the other examples.

4. Running the `hello_world` example inside the container
```bash
cd hamilton/examples/hello_world
source hamilton-env/bin/activate
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
source hamilton-env/bin/activate
source hamilton-env/bin/activate # this will activate the virtual environment

cd hamilton/examples/hello_world
source hamilton-env/bin/activate
python my_script.py
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add something like:

You should be able to run all the examples that have only a requirements.txt. If there are other requirements.txt files (like under data_quality), you'll just need to manually create an environment and install those requirements yourself.

@skrawcz skrawcz self-requested a review October 14, 2022 17:56
Copy link
Collaborator

@skrawcz skrawcz left a comment

Choose a reason for hiding this comment

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

Actually you know what, I'll just merge this as is -- since it's just documentation that I'm wanting updates to, I can go ahead and update that after merging and save you the trouble of doing it.

Thanks for your contribution!

image

@skrawcz skrawcz merged commit b4bb7eb into stitchfix:main Oct 14, 2022
@bovem
Copy link
Contributor Author

bovem commented Oct 15, 2022

Thanks @skrawcz it was fun to work on this issue. I'll keep looking on open issues to see if I could contribute on something else.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants