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): Migrate to poetry #141

Merged
merged 71 commits into from
Feb 20, 2025
Merged

(Chore): Migrate to poetry #141

merged 71 commits into from
Feb 20, 2025

Conversation

BinamB
Copy link
Contributor

@BinamB BinamB commented Oct 9, 2024

Description about what this pull request does.

Please make sure to follow the DEV guidelines before asking for review.

New Features

  • Migrated dependency handling to poetry
  • Add option to manually add a guid to urls mapping json file to validation script

Breaking Changes

Bug Fixes

  • Fix getting file name from the GDC manifest.

Improvements

  • Added support for Glacier Instant Retreival
  • Add more logs

Dependency updates

  • Update black on pre-commit
  • Update python image on Dockerfile to python3.9-buster-2.0.0
  • Update dependencies in toml

Deployment changes

NOTES: changed scripts folder to dcfdataservice. I did not pick dcf-dataservice over dcfdataservice because poetry's toml file does not like special characters like - or _

jacob50231
jacob50231 previously approved these changes Dec 18, 2024
Copy link
Contributor

@jacob50231 jacob50231 left a comment

Choose a reason for hiding this comment

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

Code looks good assuming we can get this to pass jenkins.

requirements.txt Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we're still tracking requirements.txt after migrating to poetry?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that is still needed for the Google dataflow job

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah its a different requirements file thats supposed to provide the dependencies for the google dataflow job. The poetry toml installs the dependencies on the kubernetes job and the kubernetes job initiates a google job which uses the requriements.txt

Copy link
Contributor

@MaribelleHGomez MaribelleHGomez left a comment

Choose a reason for hiding this comment

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

lgtm

jacob50231
jacob50231 previously approved these changes Feb 17, 2025
Copy link
Contributor

@jacob50231 jacob50231 left a comment

Choose a reason for hiding this comment

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

I was able to fix the snyk issue and we were able to get it tested on a dev environment. Everything is looking good!

@jacob50231 jacob50231 dismissed their stale review February 17, 2025 18:20

Premature still more tests to run

@jacob50231 jacob50231 merged commit c81a4ff into master Feb 20, 2025
4 checks passed
@jacob50231 jacob50231 deleted the chore/migrate-to-poetry branch February 20, 2025 15:11
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