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

Replace custom build system with Poetry #792

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

eecavanna
Copy link
Collaborator

@eecavanna eecavanna commented Nov 23, 2024

In this branch, I am working on replacing the current pip-tools-based build system with a Poetry-based one.

As of now, all the containers start up OK in my development environment. I have deleted the requirements directory. I have introduced Poetry and updated the Dockerfiles and all other dependent files I've come across so far, accordingly. I have not tested the behavior of the Runtime or Dagster (beyond confirming their web UIs load) and have not run the automated tests locally (they run OK on GitHub Actions, though).

Details

I'll add some details here when this branch is further along.

It seems to me that multiple programs — all of whose code is managed in this repo — share a single dependency list (meaning, there's one bucket of dependencies/packages that different applications pull from as they need something, but that bucket contains all dependencies of all programs). That may be another opportunity for simplification/cleanup here: separate the different programs (e.g. Dagster stuff, maybe eventually the standalone Minter) out into their own Poetry projects. We can discuss this.

Note: The Mongo migration notebooks do not share a dependency list/bucket with the rest of the applications in the repo. Those notebooks happen to live in the repo, but don't have any dependency upon anything in the repo outside of their own directory (they depend only on either packages defined within that directory, or packages hosted on PyPI).

Related issue(s)

Fixes #563

Related subsystem(s)

  • Runtime API (except the Minter)
  • Minter
  • Dagster
  • Project documentation (in the docs directory)
  • MongoDB migrations
  • Other

Testing

  • I tested these changes (explain below)
  • I did not test these changes

I confirmed that whatever tests GitHub Actions runs, all pass.

Documentation

  • I have not checked for relevant documentation yet (e.g. in the docs directory)
  • I have updated all relevant documentation so it will remain accurate
  • Other (explain below)

Maintainability

  • Every Python function I defined includes a docstring (test functions are exempt from this)
  • Every Python function parameter I introduced includes a type hint (e.g. study_id: str)
  • All "to do" or "fix me" Python comments I added begin with either # TODO or # FIXME
  • I used black to format all the Python files I created/modified
  • The PR title is in the imperative mood (e.g. "Do X") and not the declarative mood (e.g. "Does X" or "Did X")

@eecavanna eecavanna linked an issue Nov 23, 2024 that may be closed by this pull request
@eecavanna eecavanna self-assigned this Nov 23, 2024
@eecavanna
Copy link
Collaborator Author

I'm pleasantly surprised that the GHA workflows all passed! I'll resume looking at this PR next week.

@eecavanna
Copy link
Collaborator Author

Hi @dwinston, if we stop using the current pip-tools-based dependency management solution and, instead, use a Poetry-based one, do you know whether it will still be necessary to have the files setup.py and tasks.py in the repo?

I'm not too familiar with those files. Based on my vague memory of when you, @pkalita-lbl, @shreddd, and I worked on automating Runtime-related parts of the release process (last year, when making commit cbd58f3 and nearby commits), I believe these files came into play when building nmdc-runtime as a Python package that would be published to PyPI. I don't know whether they are used in any other situations.

@dwinston
Copy link
Collaborator

@eecavanna they are not currently used. Destroy them.

@eecavanna
Copy link
Collaborator Author

Thanks, @dwinston. I'll put them on my ✂️ list.

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.

Replace custom build system with Poetry
2 participants