Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Dockerfile: Install pydantic with --no-binary #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Dockerfile: Install pydantic with --no-binary #17

wants to merge 1 commit into from

Conversation

anthonyfok
Copy link
Member

@anthonyfok anthonyfok commented Nov 4, 2021 •
edited
Loading

This is to reduce the size of the resulting Docker image,
apparently from 969MB down to 861MB

Fixes #15

Questions: Is it a good idea to push this to master branch? Or to keep it just for a special pydantic-no-binary branch for AWS Lambda only? Many thanks!

@anthonyfok anthonyfok mentioned this pull request Nov 4, 2021
Open
@anthonyfok
Dockerfile: Install pydantic with --no-binary
87090a5 
This is to reduce the size of the resulting Docker image,
apparently from 969MB down to 861MB

Fixes OpenDRR#15
@anthonyfok anthonyfok added this to the Sprint 45 milestone Nov 4, 2021
jvanulde
jvanulde requested changes Dec 29, 2021
View reviewed changes
Copy link

@jvanulde jvanulde left a comment

Choose a reason for hiding this comment

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

I think it should be an optional setting. We can note that it's a necessity for AWS Lambda. Perhaps we should propose to the pygeoapi community as well via a PR?

@anthonyfok anthonyfok modified the milestones: Sprint 45, Sprint 52 Jan 26, 2022
@anthonyfok anthonyfok self-assigned this Jan 26, 2022
@anthonyfok
Copy link
Member Author

anthonyfok commented Jan 26, 2022 •
edited
Loading

Turns out I probably changed the wrong Dockerfile: instead of the one in the top-level directory, I probably should have changed aws-lambda/container/Dockerfile instead. (Thanks to @arashmalekz's comment #15 (comment) telling me about the aws-lambda directory which I neglected.)

See also geopython#709 for the origin of aws-lambda/container/Dockerfile.

Will revisit this in a future sprint (say Sprint 52?)

@jvanulde
Copy link

jvanulde commented Jan 26, 2022 •
edited by anthonyfok
Loading

Note that since we don't deploy on Lambda any longer we don't need to worry about this.

@anthonyfok
Copy link
Member Author

Note that since we don't deploy on Lambda any longer we don't need to worry about this.

Thanks so much Joost! I was wondering about that ('cause it was mentioned at our cloud tech meetings) but wasn't sure of the details.

I guess what is left to do in this PR is to (1) apply the change to the correct Dockerfile, and (2) submit a PR upstream, and then we can close this PR. Postponing this till we have more free time. :-)

@anthonyfok anthonyfok removed this from the Sprint 52 milestone Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jvanulde jvanulde jvanulde requested changes

@drotheram drotheram Awaiting requested review from drotheram

@arashmalekz arashmalekz Awaiting requested review from arashmalekz

Assignees

@anthonyfok anthonyfok

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Docker image build fails when the flag '--no-binary pydantic' is added to requirements.txt
2 participants
@anthonyfok @jvanulde