-
Notifications
You must be signed in to change notification settings - Fork 27
Add dockerfile to run locally #21
base: in-memory
Are you sure you want to change the base?
Conversation
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 idea, thanks @raoufchebri!
I have a couple of comments/questions, there are rather optional. So you can just merge as is if it blocks you.
rm -r /tmp/pg_embedding && \ | ||
apt-get remove -y build-essential postgresql-server-dev-$PG_MAJOR && \ | ||
apt-get autoremove -y && \ | ||
rm -rf /var/lib/apt/lists/* |
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.
Instead of cleaning up things afterwards, I would probably split this into a multi-stage build:
- build pg_embedding
- copy the extension from the previous stage to a fresh
postgres:$PG_MAJOR
container
# Locale settings | ||
RUN apt-get clean && apt-get update && \ | ||
apt-get install -y locales && \ | ||
localedef -i en_US -c -f UTF-8 -A /usr/share/locale/locale.alias en_US.UTF-8 | ||
|
||
ENV LANG en_US.utf8 |
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 curious, Is this required for the extension, or is it just to make the image more usable?
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 not sure this step is required, but I got multiple errors with locale, and this fixed it.
@bayandin Will this PR be merged? |
@samueldurantes, we have sunsetted |
No description provided.