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

chore: Make pypgstac an ENV in dockerfile #404

Merged
merged 10 commits into from
Aug 14, 2024

Conversation

sandrahoang686
Copy link
Contributor

@sandrahoang686 sandrahoang686 commented Jul 29, 2024

Issue

#372

What?

Make pypgstac and ENV var in dockerfile

Why?

  • Description of why the changes were made

Testing?

Deployed without any errors to http://monitor.openveda.cloud/

@sandrahoang686 sandrahoang686 changed the title Make pypgstac an ARG in dockerfile chore: Make pypgstac an ARG in dockerfile Jul 29, 2024
@sandrahoang686 sandrahoang686 changed the title chore: Make pypgstac an ARG in dockerfile chore: Make pypgstac an ENV in dockerfile Jul 29, 2024
@sandrahoang686 sandrahoang686 marked this pull request as ready for review July 29, 2024 19:55
docker-compose.yml Outdated Show resolved Hide resolved
ingest_api/runtime/Dockerfile Outdated Show resolved Hide resolved
@@ -8,7 +8,7 @@ orjson>=3.6.8
psycopg[binary,pool]>=3.0.15
pydantic_ssm_settings>=0.2.0
pydantic>=1.10.12
pypgstac==0.7.4
pyjwt>=2.8.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

this came from merging upstream develop changes. The same line and same first two characters confused the merge conflict resolver

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for pointing this out! I got rid of pyjwt.. because looks like it was no longer in the develop branch and added pypgstac back

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

But we still do need - pypgstac==0.7.4

@anayeaye
Copy link
Collaborator

anayeaye commented Aug 8, 2024

@sandrahoang686 I took the liberty of pulling recent changes from develop into your branch and making this small change to get the pgstac_version in the build api parameters. I can see that worked in the action because the docker build is now installing the correct version #12 [ 8/16] RUN pip install -r /tmp/ingestor/requirements.txt pypgstac==0.7.10 -t /asset --no-binary pydantic uvicorn. Now I see that we need to do the same for the ingestor construct (build_ingestor)...

@sandrahoang686
Copy link
Contributor Author

@anayeaye I've passed in pgstac_version to ingest_api => build_ingestor and added it as a build_arg to docker_build. Am i missing anything? If I wanted to test this btw, how would I do so correctly?

Copy link
Collaborator

@anayeaye anayeaye left a comment

Choose a reason for hiding this comment

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

I see the correct pypgstac installed in the github action log as well as Code updates for both the ingest api handler and the ingestor construct.

@anayeaye
Copy link
Collaborator

@anayeaye I've passed in pgstac_version to ingest_api => build_ingestor and added it as a build_arg to docker_build. Am i missing anything? If I wanted to test this btw, how would I do so correctly?

@sandrahoang686 you could do a local cdk synth and look at the outputs but I think the easiest way to verify this kind of change is to read the pre-deploy job associated with this PR. I searched to be sure that the correct pypgstac was installed in both ingest lambda Dockerfile builds.

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

Successfully merging this pull request may close these issues.

3 participants