chore: add ddtrace-internal as dependency#18969
Conversation
Codeowners resolved as |
|
Circular import analysis
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e22afe9516
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "bytecode>=0.15.1,<1; python_version~='3.12.0'", | ||
| "bytecode>=0.14.0,<1; python_version~='3.11.0'", | ||
| "bytecode>=0.13.0,<1; python_version<'3.11'", | ||
| "ddtrace-internal<1.0", |
There was a problem hiding this comment.
Add a lower bound for ddtrace-internal
This new runtime dependency only has an upper bound, but scripts/check-dependency-bounds rejects project dependencies without both lower and upper bounds and the generated “Check project dependencies” job runs that script for pyproject.toml changes. Please add the minimum ddtrace-internal version that contains the APIs ddtrace needs, for example alongside the existing <1.0 cap, so this dependency policy check does not fail and older prereleases cannot satisfy constrained installs.
Useful? React with 👍 / 👎.
| }, | ||
| command="pytest -v -n auto {cmdargs} tests/internal/", | ||
| pkgs={ | ||
| "datadog-internal": "==0.0.0rc12", |
There was a problem hiding this comment.
Use the same ddtrace-internal package in the riot venv
The product metadata added in this commit requires ddtrace-internal, but this riot environment asks pip to install a different distribution, datadog-internal. When riot run internal builds this venv, pip resolves pkgs literally, so this extra package does not pin or exercise the dependency that was added and can fail venv creation if datadog-internal is unavailable; please use ddtrace-internal here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
General question about this: I remember hearing/reading that there was a maximum size we needed not to go over for serverless to be happy with ddtrace as a dependency. If we add ddtrace-internal as a dependency, are we not potentially making the layer with ddtrace in it bigger because of duplication (I'm mainly concerned about any native dependencies ddtrace-internal would have that "normal" ddtrace would also have).
(I don't have a solution, I just wanted to raise the question 😅)
No description provided.