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.

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

Cleanup dockerfiles, remove duplicates, upgrade PDM #6554

Merged
merged 5 commits into from
Sep 9, 2024

Conversation

spwoodcock
Copy link
Member

@spwoodcock spwoodcock commented Sep 4, 2024

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation
  • πŸ§‘β€πŸ’» Refactor
  • βœ… Test
  • πŸ€– Build or CI
  • ❓ Other (please specify)

Related Issue

Fixes #6552

Describe this PR

  • Removes dup / redundant dockerfiles.
  • Upgrades PDM and debugpy to latest.
  • Removes hardcoded --workers flag for Gunicorn (can be set by WEB_CONCURRENCY env var instead).
  • Re-locks dependencies after PDM version upgrade using inherit_metadata strategy (required PDM > 2.15).

Review Guide

Notes for the reviewer. How to test this change?

Checklist before requesting a review

[optional] What gif best describes this PR or how it makes you feel?

Dockerfile Outdated
Comment on lines 37 to 42
RUN apt-get update && apt-get install --no-install-recommends -y \
build-essential \
libffi-dev \
libgeos-dev
libgeos-dev \
postgresql-server-dev-15 \
python3-dev

Check notice

Code scanning / SonarCloud

Arguments in long RUN instructions should be sorted Low

Sort these package names alphanumerically. See more on SonarCloud
Copy link

sonarcloud bot commented Sep 4, 2024

@mahesh-naxa
Copy link
Collaborator

@dakotabenjamin i just verified this, can be merged.

@spwoodcock spwoodcock merged commit d639285 into hotosm:develop Sep 9, 2024
7 checks passed
@dakotabenjamin
Copy link
Member

dakotabenjamin commented Sep 9, 2024

Errors after merging, which weren't caught because they are in server instantiation:

+ sudo apt-get -q -y install libgeos-3.9.0 libgeos-dev
Reading package lists...
Building dependency tree...
Reading state information...
E: Unable to locate package libgeos-3.9.0
E: Couldn't find any package by glob 'libgeos-3.9.0'
E: Couldn't find any package by regex 'libgeos-3.9.0'
+ sudo apt-get -q -y install libproj19 libproj-dev
Reading package lists...
Building dependency tree...
Reading state information...
E: Unable to locate package libproj19

In addition, a slew of errors compiling sklearn during pip install.
edit: and a warning that might be related:

/tmp/pip-build-env-ao16q_16/overlay/lib/python3.8/site-packages/scipy/__init__.py:143: UserWarning: A NumPy version >=1.19.5 and <1.27.0 is required for this version of SciPy (detected version 1.17.3)
      warnings.warn(f"A NumPy version >={np_minversion} and <{np_maxversion}"

@spwoodcock
Copy link
Member Author

Any way we can test the Cloudformation (I assume) stuff locally?

It's a bit baffling how the libgeos install would have broken when this PR updates dockerfiles 😯

The second issue around the numpy version can be fixed by pinning the numpy version explicitly (if sklearn requires this)

@mahesh-naxa
Copy link
Collaborator

@spwoodcock We can use https://www.localstack.cloud/ to run the EC2 instance with the defined user-data scripts locally, but i am not sure if it supports the AMI we are using. Let me check.
Otherwise i will spin up a EC2 in the AWS to validate everything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

scripts/docker/Dockerfile.backend deprecated?
3 participants