-
Notifications
You must be signed in to change notification settings - Fork 1
Feat: improve proof-of-concept app #116
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
Conversation
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
b32158d
to
31a45b5
Compare
271fa52
to
394f3da
Compare
<h2>Map</h2> | ||
<div class="row" style="min-height: 450px;"> | ||
<div class="col-lg-12 border"> | ||
<iframe class="streamlit-app" src="{{ streamlit_host }}/stations--stations?embed=true&district_number={{ current_district.number }}"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename class to streamlit-service
Realized that the custom streamlit-app
class was not necessary since it can be replaced by Bootstrap's w-100 h-100
.
pems/core/context_processors.py
Outdated
def streamlit_host(request): | ||
"""Context processor to add the Streamlit host URL part to the context.""" | ||
|
||
return {"streamlit_host": settings.STREAMLIT_HOST} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider top-level streamlit
context
pems/settings.py
Outdated
|
||
# Streamlit settings | ||
STREAMLIT_LOCAL_PORT = os.environ.get("STREAMLIT_LOCAL_PORT", "") | ||
STREAMLIT_HOST = os.environ.get("STREAMLIT_HOST", f"http://localhost:{STREAMLIT_LOCAL_PORT}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
collapse these 2 variables to 1
pems/static/css/styles.css
Outdated
@@ -10,3 +10,8 @@ header { | |||
} | |||
} | |||
} | |||
|
|||
.streamlit-app { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a class in Bootstrap already that already sets these 2 properties? if so, use that one
@@ -35,6 +35,8 @@ EXPOSE 8501 | |||
|
|||
COPY streamlit_app streamlit_app | |||
|
|||
COPY .streamlit .streamlit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this to before COPY streamlit_app streamlit_app
to take advantage of layer caching
2af3987
to
1e95879
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to run this locally without any issues, thank you for the great write up!
Since I still need to renew my DOT account password, I didn't run anything against AWS. But the changes look good 👍
The only small comment would be to remove the pems/streamlit_sample
app and corresponding streamlit_app/apps/sample
since you have a real streamlit app now. No worries either way.
0864cb8
to
17aa7a4
Compare
@lalver1 thanks for removing the sample app! Do you want to try to squash that |
Thanks for the review @thekaveman! That was a good suggestion to remove the |
the District Stations Streamlit app is a simple proof-of-concept that displays the traffic data collection stations in a district.
the django and streamlit apps are now wired up in a way that works both for a local dev environment as well as a Cloud deployment.
since the web and streamlit services run on the same load balancer in AWS, the http path defined in the manifest of each service must be different from each other. the http path of the streamlit service (streamlit_service) is also passed to the streamlit run command in the entrypoint.sh script. also, the healthcheck path must be defined in the manifest so that the service does not continually restart.
this setup was causing an error when deploying to AWS since the Django pems app resides in a different container. when running a local debugger though, this error was not present since the local dev container does have the pems app. since at this moment we are not using Django models in the streamlit app, we are removing this ability.
copy the .streamlit directory into the streamlit service's image so that the streamlit configuration is available to the container. before, the configuration was only available to the local dev container.
for AWS copilot deployments, load the fixtures from S3 on each deploy: 1. add the pems-db addon to the web service 2. set up the aws CLI in the app container (remove it from the dev container since it'll already be available) 3. run start_aws.sh using the web service manifest's CMD definition (local development will still use start.sh in CMD)
also remove sample app from streamlit_app since we are not using it anymore.
since the streamlit_sample Django app was removed, the load balancer can now use /streamlit as the path to serve the streamlit copilot service and not worry about path naming conflicts.
e29a2b0
to
3578cfd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work 🚀 🚀
I left a few comments for follow-up, but this looks great for now!
http: | ||
# Requests to this path will be forwarded to your service. | ||
# To match all requests you can use the "/" path. | ||
path: "/streamlit" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 😎
@@ -52,6 +52,23 @@ RUN useradd --create-home --shell /bin/bash $USER && \ | |||
apt-get install -qq --no-install-recommends build-essential nginx gettext && \ | |||
python -m pip install --upgrade pip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's take a follow-up to add the Buildkit cache mounting feature to these Dockerfiles, for apt
, pip
, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, I was also thinking that I should add this feature soon 😄
platform: linux/x86_64 # See https://aws.github.io/copilot-cli/docs/manifest/lb-web-service/#platform | ||
count: 1 # Number of tasks that should be running in your service. | ||
exec: true # Enable running commands in your container. | ||
command: bin/start_aws.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see 👍
|
||
echo "Downloading $S3_FIXTURE_PATH from bucket $S3_BUCKET_NAME" | ||
aws s3 cp "s3://${S3_BUCKET_NAME}/${S3_FIXTURE_PATH}" "${LOCAL_FIXTURE_PATH}" | ||
echo "Download complete" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a follow-up, let's look into mounting the bucket as a volume that presents as a readable directory in the container. I assume this is possible with our setup (we do something similar in Azure).
This seems good for now 👍
def streamlit(request): | ||
"""Context processor adds Streamlit-related information.""" | ||
|
||
return {"streamlit": {"url": settings.STREAMLIT_URL}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Closes #114
Adds the
stations
Streamlit app to the repo (before it was only a local branch) and wires up Django and Streamlit in an environment-dependent way so that the proof-of-conceptdistricts
app can run both locally and in the Cloud.Currently this branch is on top ofci/cloud-infra
but it will be rebased onmain
once PR #111 is merged.Notes
AWS Copilot services
Originally, the
streamlit
service was defined as a copilotBackend Service
. For an initial proof-of-concept this is not a convenient service type since the service is unreachable from the internet (recall that theweb
service embedsstreamlit
in an iframe, sostreamlit
needs to be reachable from the internet). For this PR, wewill changechangedstreamlit
to aLoad Balanced Web Service
. As we get closer to production, the architecturewill likelymay change to:web
to act as a reverse proxyiframe
s inweb
will point to a path inweb
but this path will be set up in thengingx
configuration to forward any requests tostreamlit
over the private networkstreamlit
in a private network (back toBackend Service
) and configure it to deny all incoming traffic except for requests from the private IP address ofweb
.Django database fixtures
For AWS copilot deployments, we load the fixtures from S3 on each deploy by:
Adding the
pems-db
addon to theweb
serviceThis S3 bucket addon was created using
and the output of the command was an addon manifest file. The S3 bucket's lifecycle is tied to the
web
service, so it will be deleted if theweb
service is deleted. Once the deployment process stabilizes, the S3 bucket's lifecycle will instead be tied to the copilot environment.Setting up the
aws
CLI in the app containerThe
aws
CLI will be needed for step 3, so it is installed in the app container. There is no need to re-install it in the dev container so the installation step in the dev container's Dockerfile has been removed.Running
start_aws.sh
using theweb
service manifest'sCMD
definitionSince the
web
service manifest'sCMD
definition overrides the Dockerfile'sCMD
, we use this feature to runstart_aws.sh
which copies the fixtures from the S3 bucket and loads them into the SQLite database, while still keeping the original behavior for local builds of the app container (local development will still usestart.sh
inCMD
)Reviewing
Local behavior
To ensure that all the compose services still work locally, you can run
bin/build.sh
followed by starting up the dev container. Then you can start a debugging session for bothDjango: PeMS Client
andStreamlit, no sidebar
and see the locally running app in your browser.AWS behavior
Running
from inside the
infra
folder redeploys the services to AWS. Currently thedev
environment is running at http://pems-d-publi-tdvc9tnwbm5n-1785238641.us-west-2.elb.amazonaws.com/ but it can change at any time until the environment is stabilized.