-
Notifications
You must be signed in to change notification settings - Fork 63
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
Add Docker Compose support with nginx reverse proxy. #60
base: main
Are you sure you want to change the base?
Add Docker Compose support with nginx reverse proxy. #60
Conversation
Set up docker-compose.yaml to create all necessary resources to run all Streamlit apps at the same time and reverse proxy them all using nginx. Add basic HTML file for nginx to serve that provides links to all the different apps.
Whoa! This looks neet! I'll give it a proper review first thing tomorrow! |
README.md
Outdated
git clone git@github.com:RasaHQ/rasalit.git | ||
cd rasalit | ||
|
||
RASA_PROJECT_DIR=/path/to/my_rasa_project OVERVIEW_FOLDER=/path/to/gridresults docker-compose up -d |
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.
This could be simplified if there were a GitHub action to build all of these containers and push them up to DockerHub, in which case you wouldn't need the whole Rasalit repository and to build the containers yourself (which takes a while). In this case, you would only need to curl the docker-compose.yaml
file from raw.githubusercontent.com and pull the pre-built containers from DockerHub (e.g. RasaHQ/rasalit-overview
, RasaHQ/rasalit-spelling
, etc.).
Strange. When I try to connect to localhost:8000 I get no response. After running docker compose it also seems like no port is opening up as this command gives nothing.
I think there's something happening in the
|
It seems like the
|
The nginx container will continue restarting until all the other containers are available. In this case the If you run |
docker-compose.yaml
Outdated
duckling: | ||
image: rasa/duckling | ||
container_name: rasalit-duckling |
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'm wondering if we need this around. If you're running duckling you should already have a config.yml file that is pointing to it. Is there a reason why we need it?
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 suppose not everyone has duckling in their NLU pipeline - I was using the financial-demo example which does have duckling, and the duckling server needs to be available to the "spelling" and "live-nlu" apps because they use RasaNLUInterpreter
(if I understand correctly).
If you've got a duckling server running elsewhere on your machine, you can tell the apps where duckling is via the RASA_DUCKLING_HTTP_URL
environment variable but the duckling server would need to be visible to the containers in the Rasalit Docker Compose, which means either using an external network or just using host networking in all of the Rasalit Docker Compose containers.
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 fear things might start getting tricky here.
If the duckling URL changes then the config.yml
will require a retrain before you have a model that you can pass to rasalit. That means that suddenly you may need to train a model again (which is expensive) if you want to check it out via the docker-compose trick.
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 under the impression that the RASA_DUCKLING_HTTP_URL
always took precedence if present, meaning you don't need to retrain the model if the duckling URL changes - RasaHQ/rasa#6240
Ah right, I think I found the issue. The current set up assumes that I ran a grid-search beforehand. If there's no existing gridsearch folder passed along, we get this:
|
It looks to me like it's searching for the built Rasa models from the The gridresults will also be an issue, but that should only be an issue for the |
That is the base assumption yes. When you train a Rasa model locally it saves a |
Part of me is thinking "maybe it's good to take a brief moment to take a step back". I never designed these streamlit apps to be bundled together in a single service. I imagined they'd be running as one-off inspection moments locally. I really like the direction that this PR is considering, but it does occur to me that it'd be good to ponder about/double-check the edge cases. One idea that I'm contemplating (feel free to shoot at the idea): would it make sense to have people configure their own I'm mainly mentioning this because of maintainability. I maintain about 10+ small open-source projects and I've always found that by passing responsibility to the end-user, you typically empower them which results in less issues. |
Yeah that makes sense, I can't think of why they would need to change the |
Yeah, you're right. Any environment variables should be passed through Given this situation there now seem to be three options with regards to implementation.
I'm still very much in the "thinking out loud-mode" so feel free to challenge me on this idea. Do you have any concerns for option 3? Option 3 would involve adding a command to the CLI written with typer. Should you be unfamiliar, you may appreciate a tutorial that I wrote on calmcode. |
Sorry for the delay, I've not been able to implement this yet but option 3 sounds like the right choice to me, and shouldn't be too difficult to do. I'll try to find some time at some point to refactor this and add the Typer command to generate the starter compose YAML. |
I took a stab at adding a command to generate the The solution would be to add a GitHub Actions workflow to generate the Docker images and publish them to GitHub Container Registry. I don't think this would be too difficult but not sure whether you think that would be better put in another PR. |
I'm awaiting the release of Rasa 3.0 before adding features to this repo. The main reason is that I want to be 100% sure that the apps themselves are at least compatible with the most recent Rasa version. |
Fair enough, I'll put up a PR for a simple GH action to publish the images but understand this PR or the other one won't be merged until a later date - just thought I may as well do it while it's fresh in my mind. |
Set up docker-compose.yaml to create all necessary resources to run all Streamlit apps at the same time and reverse proxy them all using nginx.
Add basic HTML file for nginx to serve that provides links to all the different apps.
This is a very simple reverse proxy integration, so there's no "back" buttons once you navigate to a particular Streamlit app.
See #58 for context.