-
Notifications
You must be signed in to change notification settings - Fork 30
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
[agentrunner] Split the docker image and clean up the logs #404
Conversation
<groupId>io.netty</groupId> | ||
<artifactId>netty-transport-native-unix-common</artifactId> | ||
<version>${netty.version}</version> | ||
</dependency> |
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.
remove ?
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 is commented code
@@ -58,8 +27,8 @@ ENV NLTK_DATA="/app/nltk_data" | |||
|
|||
# Install python runtime deps | |||
RUN cd /app && pipenv requirements --categories="packages full" > /app/requirements.txt \ | |||
&& python3 -m pip install -r /app/requirements.txt \ | |||
&& python3 -m nltk.downloader -d /app/nltk_data punkt averaged_perceptron_tagger | |||
&& python3 -m pip install -r /app/requirements.txt \ |
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.
since this stuff takes a lot and it's likely to never change during development, can we put it in the base image ?
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.
Good point
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.
but we cannot do it
because of this line, that depends on the langstream-runtime build (the "maven" directory)
ADD maven/Pipfile.lock /app/Pipfile.lock
cc @cbornet
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 think we should isolate the python stuff in it’s own maven subproject.
Summary:
Changes to the docker images:
Changes to logging: