Support exporting python dependency metadata#89
Conversation
This change introduces changes to support exporting information about the python dependencies used by a project in two different forms: - In the component itself in the producers section; this is part of the data typically injected by the fastly cli. - Via a separate subcommand; this is mostly useful for testing. Other languages today that include dependency information have that information extract by go code in the fastly cli directly. That is where I started, but there were issues given the variety of ways that python dependencies are managed (in virtualenvs, etc.). Ultimately, I decided that the optimal solution would be to get this information at the time of the build. Integration with the CLI is a seprate task, but the basic idea for python is that it will parse out the dependency metadata from the componennt if present (and not overwrite it) and merge it with the other metadata it wants to add. The dependencies subcommand was another approach considered, but it required other hacks to try to guess at how to correctly run fastly-compute-py based around parsing of the build command -- while this might work, it might also fail catastrophically if customers invoke make or other similar sorts of things. Pushing things into the build incrases our changes of consistenlty and correctly capturing this metadata. For now, this data is only inluded if targeting UV, but it should be possible to expand support if we have other heuristics to gather PEP751 compatible version information.
| /// Errors are bubbled up when UV is known to be available (i.e. the `UV` | ||
| /// environment variable is set, which `uv run` always provides). If UV is | ||
| /// not discoverable at all we warn and return an empty list, since the project | ||
| /// may not be using UV as its package manager. |
There was a problem hiding this comment.
I'm a bit concerned about the fragility of this - the project may be using Poetry or PDM as its package manager/build system but uv may be available in the PATH. If we need this information, I think we should consider mandating that uv be used for fastly-compute-py projects, just as the C++ SDK mandates that CMake be used.
There was a problem hiding this comment.
Here we're checking the UV environment variable, not uv on the path. This env var is set when running via a uv command but would be very unusual for it to be set otherwise.
It was this sort of path-checking logic that led me down this path over other heuristics.
There was a problem hiding this comment.
OK, thanks for the explanation, I did not read closely enough :-)
There was a problem hiding this comment.
As far as requiring uv, right now I'm thinking we encourage it and make it the paved path but I don't think we need to require it. Here I try to target standards (pylock is the PEP751 accepted standard) but tools like poetry don't support it yet (though they seem to acknowledge they ought to https://github.com/orgs/python-poetry/discussions/10322).
I consider including this dependency info a "nice-to-have" and probably not worth dropping. I'm open to changing my opinion as well -- I think uv has staying power but there's also been a long history of python build tools so I'm not too keen on fully betting on a single horse.
There was a problem hiding this comment.
And the company behind uv was just acquired by OpenAI, so who knows where that wil lead.
posborne
force-pushed
the
posborne/python-dependencies-metadata
branch
from
7c17be4 to
c5a732f
Compare
|
Apologies for the force-push; I had an amended commit with the format fixes that I must not have force-pushed before opening this PR yesterday. |
| """Pretend to patch.""" | ||
| print("Faking the run of exception-mapping monkeypatches for test runner.") | ||
| import sys | ||
| print("Faking the run of exception-mapping monkeypatches for test runner.", file=sys.stderr) |
There was a problem hiding this comment.
This can stay or go, I just changed this as it was causing grief trying to write tests against the cli output.
This fixes test failures where we are using uv without a project for viceroy tests and is a reasonable heuristic. Requiring uv.lock would be a step further, but this may or may not be present when using uv depending on how it is used.
Stdout ends up with logging mixed in today, so for testing and machine use of JSON dependencies output, support specifying an output file.
| Uses --output to write JSON to a temp file, avoiding any ambiguity from | ||
| uv or logging infrastructure writing to stdout. | ||
| """ | ||
| with tempfile.TemporaryDirectory() as tmp_dir: |
There was a problem hiding this comment.
Nit: There's a pytest fixture which can give you a temporary directory.
There was a problem hiding this comment.
Ah, nice, in this case I think it makes sense to keep with the context manager within this function as we callers don't need to access the temporary file. We're just using that path to avoid log's written to stdout from messing up the JSON output from the command.
2 participants
This change introduces changes to support exporting information about the python dependencies used by a project in two different forms:
Other languages today that include dependency information have that information extract by go code in the fastly cli directly. That is where I started, but there were issues given the variety of ways that python dependencies are managed (in virtualenvs, etc.). Ultimately, I decided that the optimal solution would be to get this information at the time of the build.
Integration with the CLI is a seprate task, but the basic idea for python is that it will parse out the dependency metadata from the componennt if present (and not overwrite it) and merge it with the other metadata it wants to add.
The dependencies subcommand was another approach considered, but it required other hacks to try to guess at how to correctly run fastly-compute-py based around parsing of the build command -- while this might work, it might also fail catastrophically if customers invoke make or other similar sorts of things. Pushing things into the build incrases our changes of consistenlty and correctly capturing this metadata.
For now, this data is only inluded if targeting UV, but it should be possible to expand support if we have other heuristics to gather PEP751 compatible version information.
I do have a version of fastly cli changes that will work along with this; given that we haven't publicly opened up python support as of yet, I haven't pushed those changes anywhere of yet.