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

npx nx generate @nxlv/python:uv-project produces root pyproject.toml #258

Closed
ahammond opened this issue Dec 16, 2024 · 19 comments · Fixed by #260
Closed

npx nx generate @nxlv/python:uv-project produces root pyproject.toml #258

ahammond opened this issue Dec 16, 2024 · 19 comments · Fixed by #260
Labels
enhancement New feature or request

Comments

@ahammond
Copy link

ahammond commented Dec 16, 2024

Current Behavior

NX  Generating @nxlv/python:uv-project

CREATE apps/lambda-demo/project.json
CREATE apps/lambda-demo/README.md
CREATE apps/lambda-demo/.python-version
CREATE apps/lambda-demo/lambda-demo-module/__init__.py
CREATE apps/lambda-demo/lambda-demo-module/hello.py
CREATE apps/lambda-demo/pyproject.toml
CREATE apps/lambda-demo/tests/__init__.py
CREATE apps/lambda-demo/tests/conftest.py
CREATE apps/lambda-demo/tests/test_hello.py
CREATE pyproject.toml
  Updating root uv.lock...
error: Workspace member `/Users/andrewhammond/Documents/ClickUp/clickup/apps/activity-service` is missing a `pyproject.toml` (matches: `apps/*`)

Expected Behavior

Should not do things outside it's own app/lambda-demo-module directory.

Steps to Reproduce

  1. in an NX repo with pre-existing apps, attempt to generate
  2. notice presence of root level pyproject.toml file which breaks non-python apps in monorepo

Nx Report

❯ npx nx report

 NX   Report complete - copy this into the issue template

Node           : 20.16.0
OS             : darwin-arm64
Native Target  : aarch64-macos
pnpm           : 9.15.0

nx                 : 19.8.14
@nx/js             : 19.8.14
@nx/jest           : 19.8.14
@nx/linter         : 19.8.14
@nx/eslint         : 19.8.14
@nx/workspace      : 19.8.14
@nx/devkit         : 19.8.14
@nx/esbuild        : 19.8.14
@nx/eslint-plugin  : 19.8.14
@nx/nest           : 19.8.14
@nx/node           : 19.8.14
@nx/plugin         : 19.8.14
@nrwl/tao          : 19.8.14
@nx/vite           : 19.8.14
@nx/web            : 19.8.14
@nx/webpack        : 19.8.14
typescript         : 5.6.2
---------------------------------------
Registered Plugins:
./tools/nx/plugins/jest-shard.ts
./tools/nx/plugins/jest-shard.ts
./tools/nx/plugins/jest-shard.ts
./tools/nx/plugins/jest-shard.ts
./tools/nx/plugins/jest-shard.ts
./tools/nx/plugins/jest-shard.ts
./tools/nx/plugins/e2e-atomizer.ts
./tools/nx/plugins/e2e-atomizer.ts
@nxlv/python
@nx-tools/nx-container
@clickup/nx-plugin-monolith-testing
@clickup/nx-plugin-config-targets
---------------------------------------
Community plugins:
@jscutlery/semver      : 5.2.2
@nx-go/nx-go           : 3.2.0
@nx-tools/nx-container : 6.1.1
@nxlv/python           : 20.1.0
ngx-deploy-npm         : 7.1.0
---------------------------------------
Local workspace plugins:
         @clickup/nx-plugin-monolith-testing
         @clickup/nx-plugin-config-targets
         @clickup/nx-plugin-esbuild
         @clickup/nx-plugin-webpack
         @clickup/config-plugin
         @clickup/gateway-plugin
         @clickup/app-template


### Failure Logs

_No response_

### Additional Information

_No response_
@ahammond ahammond added bug Something isn't working needs-triage This issue needs triage labels Dec 16, 2024
@ahammond
Copy link
Author

This was from an unexpected root level pyproject.toml. Problem went away when I cleaned it up.

@ahammond ahammond reopened this Dec 16, 2024
@ahammond ahammond changed the title npx nx generate @nxlv/python:uv-project affects apps outside the app being generated npx nx generate @nxlv/python:uv-project produces root pyproject.toml Dec 16, 2024
@ahammond
Copy link
Author

And... now I know where the pyproject.toml file comes from! It reappears when I generate.

@lucasvieirasilva
Copy link
Owner

Hey @ahammond the root pyproject.toml is required to work in the Uv workspace as documented here: https://docs.astral.sh/uv/concepts/projects/workspaces/

Is the activity-service project not a python project?

If not, you could add exclude = ["apps/activity-service"] to the root pyproject.toml

image

@ahammond
Copy link
Author

I'm trying to add a containerized python lambda into an NX monorepo which already hosts a few hundred containerized node services. I can't reasonably follow an exclude pattern there. I could maybe have a separate directory instead of apps, but I would want it to be lambdas, not python-lambdas. I'll take a look and see if I can figure out a reasonable solution here.

@lucasvieirasilva
Copy link
Owner

Got it, i guess you could add your python projects in a different folder (e.g apps/python) and change the root pyproject members to check only for this folder, instead of apps/*

@lucasvieirasilva
Copy link
Owner

@ahammond I was playing with the uv workspaces, and I've noticed you can ignore the entire folder in the exclude property.

However, the way the uv workspace works, if you have a subfolder, like (e.g. packages/python/shared/lib1) the members = ["packages/python/*"] will raise the same issue

error: Workspace member `/home/lucas/Projects/uv-tests/packages/python/shared` is missing a `pyproject.toml` (matches: `packages/python/*`)

Which is pretty annoying btw.

So, what I've noticed if you configure your root pyproject in this way:

[tool.uv.workspace]
members = [
  "packages/python/*",
  "packages/python/shared/*",
]
exclude = [
  "packages/python/shared"
]

it will only look for uv projects in the packages/python folder and ignore all the other projects,

And if you have a subfolder in the python folder, you need to exclude the subfolder and explicitly add it to the members list.

I guess that it is worth opening an issue in the uv github repo, because it seems wrong to me.

Please let me know your thoughts about it.

@lucasvieirasilva lucasvieirasilva added help wanted Extra attention is needed and removed bug Something isn't working needs-triage This issue needs triage labels Dec 17, 2024
@ahammond
Copy link
Author

I'm thinking we'll just explicitly add every project we create instead of using any wildcards.

@lucasvieirasilva
Copy link
Owner

That could a good option for your case, since your monorepo is mostly js libs/apps

@avadhanij
Copy link

Hey @lucasvieirasilva .... thanks for the tip. This discussion essentially requires us to think about how we may want to organize code for a Python monorepo and I honestly don't know yet. This is the first and until we see a few more, it's difficult to establish a pattern.

Having said that, it's an active discussion topic in both poetry and uv.

  1. Poetry - Support subprojects in a poetry project python-poetry/poetry#2270
  2. UV - Discussion: uv workspaces in a monorepo - thoughts on change-only testing astral-sh/uv#6356

Poetry seems ahead right now. My question for you though is - In the case of uv, would it be possible to treat each nx project, i.e., each folder that contains Python code to be its own workspace?

It seems possible with poetry and so I am wondering why we can't do it with uv. For code sharing, we will need to leverage path based dependencies which both uv and poetry support.

@ahammond
Copy link
Author

That seems like a reasonable approach! @avadhanij would this avoid us having a root level pyproject.toml? While the shared dev deps are nice, I'm not loving the idea of adding every python project in multiple places. For context, here's what our root level pyproject.toml looks like now:

[project]
name = "nx-workspace"
version = "1.0.0"
dependencies = [
  "lambda-aurora-user-rotation",
]

[dependency-groups]
dev = [
  "ruff>=0.8.2",
  "autopep8>=2.3.1",
  "pytest>=8.3.4",
  "pytest-sugar>=1.0.0",
  "pytest-cov>=6.0.0",
  "pytest-html>=4.1.1"
]

[tool.uv.sources.lambda-aurora-user-rotation]
workspace = true

[tool.uv.workspace]
members = [
  "apps/lambda-aurora-user-rotation/",
]

That's with a single python lambda. Looks like we'd have to add it to

  • dependencies
  • workspace.members
  • tool.uv.sources.(new python thing) workspace = true

@ahammond
Copy link
Author

The more I read about workspaces, the more I believe that each nx app should be it's own uv workspace.

@lucasvieirasilva
Copy link
Owner

Hmm, I'm not sure about that, the workspace helps to centralize the virtual environment, dev libraries, also more importantly, share the local libraries in easy way,

If each app has its own workspace, you would need to install the dependencies for each app,

Imagine installing ruff, pytest, autopep for several different apps, that would be bad for performance and also it would consume a lot of disk space.

Nx with Js projects works in the same way, you don't need to have node_modules for each app with all the dependencies, even if you use npm workspaces, some dependencies would be installed at the root level.

@avadhanij
Copy link

avadhanij commented Dec 18, 2024

Hah, I agree. I was just expressing my apprehension to @ahammond privately.

Couple of questions for you, and mainly because I am new to nx.

  1. Can this plugin infer dependencies between projects. e.g. if app1 depends on lib1, and if code is changed in lib1, will nx affected identify correctly that app1 now needs to be rebuilt? Does it infer it from the sub-pyproject.toml file?
  2. If it can't infer, would we need to list dependencies between code in two places - pyproject.toml and project.json files?

@lucasvieirasilva
Copy link
Owner

About the root pyproject that you mentioned before, you don't need to keep adding them at the root pyproject if you place them in a different subfolder.

I know it's not pretty to have a "apps/python" folder, but that would avoid the need to add them individually to the root pyproject,

And uv should also improve their pattern to not force that every single folder in the member pattern is a uv project, it could be a docs folder that has nothing to with python.

I'm going to find some time tomorrow to raise an issue in their repo asking about that.

But until there, we had those 2 options,

Add the members individually as you're doing.

Or adding a different subfolder.

@lucasvieirasilva
Copy link
Owner

lucasvieirasilva commented Dec 18, 2024

Hi @avadhanij, yes, the plugin communicates to nx the dependency graph between the python projects, and if your lib1 changes, the app1 would be affected.

You can run "nx graph" to see all the dependency graph.

@lucasvieirasilva
Copy link
Owner

Really appreciate all the feedback, and testing for real the uv support so quickly.

@ahammond
Copy link
Author

My major hesitation about sharing a lock file is the inevitable legacy app which demands an ancient version of a dependency and then holds back the upgrade for other apps. Maybe the answer there is to just bite the bullet and upgrade the legacy apps, but that’s never worked out for me in the past: we have always ended up forking things out or otherwise getting ugly.

Huge appreciation for your responsiveness though! It’s really great to have this level of collaboration!

@lucasvieirasilva
Copy link
Owner

That is fair point, I could mimic the way I did for Poetry and support both approaches, with and without shared virtual environment.

It shouldn't be a big change from the existing code,

I'll let you guys know once I have this supported.

@lucasvieirasilva
Copy link
Owner

lucasvieirasilva commented Dec 18, 2024

Hey @ahammond, @avadhanij I just merged a PR #260 to support uv projects without the workspace feature..

You should start from scratch, delete the Python projects from your monorepo and start over.

The uv-project generator default behavior now is creating the project without the root pyproject, and each Python project will have its own .venv,
FYI: the feature to reuse the local projects still working as usual.

If you decide to move to the workspace mode later you just need to run this command npx nx g @nxlv/python:migrate-to-shared-venv --packageManager uv and it will migrate all the files to the workspace approach automatically.

Please let me know if you have any questions.

@lucasvieirasilva lucasvieirasilva added enhancement New feature or request and removed help wanted Extra attention is needed labels Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants