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

(Chore): Migrate to poetry #141

Open
wants to merge 66 commits into
base: master
Choose a base branch
from
Open

(Chore): Migrate to poetry #141

wants to merge 66 commits into from

Conversation

BinamB
Copy link
Contributor

@BinamB BinamB commented Oct 9, 2024 •
edited
Loading

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
View reviewed changes
Copy link

@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

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

MaribelleHGomez
MaribelleHGomez requested changes Dec 18, 2024
View reviewed changes
dcfdataservice/deletion.py Show resolved Hide resolved
dcfdataservice/google_replicate.py Outdated Show resolved Hide resolved
dcfdataservice/utils.py Outdated Show resolved Hide resolved
MaribelleHGomez
MaribelleHGomez previously approved these changes Jan 14, 2025
View reviewed changes
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacob50231 jacob50231 jacob50231 left review comments

@MaribelleHGomez MaribelleHGomez MaribelleHGomez left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@BinamB @MaribelleHGomez @jacob50231