Skip to content

v7.113.0-dev #634

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

v7.113.0-dev #634

wants to merge 2 commits into from

Conversation

joeyzhao2018
Copy link
Contributor

@joeyzhao2018 joeyzhao2018 commented Jul 17, 2025

What does this PR do?

Tag current code with a devN version indicating developing for next release.

Motivation

This is needed for APM System testing.

Testing Guidelines

Additional Notes

PEP 440 normalization: The standard requires that development releases use the .devN format (where N is a number, defaulting to 0 if not specified)

Types of Changes

  • Bug fix
  • New feature
  • Breaking change
  • Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

  • This PR's description is comprehensive
  • This PR contains breaking changes that are documented in the description
  • This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
  • This PR impacts documentation, and it has been updated (or a ticket has been logged)
  • This PR's changes are covered by the automated tests
  • This PR collects user input/sensitive content into Datadog
  • This PR passes the integration tests (ask a Datadog member to run the tests)

@joeyzhao2018 joeyzhao2018 requested review from a team as code owners July 17, 2025 15:27
@joeyzhao2018 joeyzhao2018 requested a review from florentinl July 17, 2025 15:28
PEP 400 requires it to be devN
@joeyzhao2018 joeyzhao2018 force-pushed the release/v7.113.0-dev branch from be15f15 to e26ab24 Compare July 17, 2025 15:41
@duncanista
Copy link
Contributor

Not sure why we have to make this change on our repo.

If you asked me, the APM System Tests should take care of this by the step they're downloading the repo to build it (I suppose).

This brings two problems:

  1. Now, everytime we release, we'd have to raise another PR which removes the .devN from the version.
  2. After releasing a new version, we'd then have to raise yet another PR to add back this .devN

I propose we do this in the APM System Tests side.

@joeyzhao2018
Copy link
Contributor Author

Not sure why we have to make this change on our repo.

If you asked me, the APM System Tests should take care of this by the step they're downloading the repo to build it (I suppose).

This brings two problems:

  1. Now, everytime we release, we'd have to raise another PR which removes the .devN from the version.
  2. After releasing a new version, we'd then have to raise yet another PR to add back this .devN

I propose we do this in the APM System Tests side.

Not sure why we have to make this change on our repo.

If you asked me, the APM System Tests should take care of this by the step they're downloading the repo to build it (I suppose)

This brings two problems:

  1. Now, everytime we release, we'd have to raise another PR which removes the .devN from the version.
  2. After releasing a new version, we'd then have to raise yet another PR to add back this .devN

I propose we do this in the APM System Tests side.

I first need to admit that i actually need to learn more about APM System tests. But based on the meeting with @florentinl, my preliminary understanding is that those tests are using very limited resources and we are trying to make thing more directly accessible in the pipeline. For that we need to do this and also upload the layer files.
But since you asked here, I'll double check with @florentinl and see if your proposed alternative is feasible and get back here.

@florentinl
Copy link
Contributor

florentinl commented Jul 18, 2025

Hey @duncanista and @joeyzhao2018,

The reason I need this is entirely system-test related, and I don't want to impose a burden on the release process of this repo for something that only appsec will ever use (I assume). Let me explain why I need this and we can probably find a better solution 😅

system-tests work using manifests that declare the first version of a library that pass a compliance test. They look like this: https://github.com/DataDog/system-tests/blob/main/manifests/python.yml

To know which tests to run or not (or more accurately to declare as XFailed or XPassed) it compares the version of the running library against this manifest. To do this it uses the self reported version of the package (datadog_lambda.__version__ for us).

Without pre-release version markers, If the main branch passes a new test there is currently no way to mark the test as passing until the next release.
For example if after a release (let say v7.112.0), a new commit makes a new test pass, I can't mark it as passed for v7.112.0 because running system-tests on the latest release would fail and I can't mark it as passed for v7.113.0 because the test wouldn't be executed for the current main and a regression in a successive commit regarding this test will not be catched (also v7.113.0 might never exist because of a breaking change and having a non existing version in a manifest is not very satisfying)

Having a pre-release marker solves the issue by allowing to mark a test as passing starting v7.113.0.dev and it would run on main but not on the latest release.

If modifying the version manually inside the repo is too much of a burden, maybe we can try to automate it. dd-trace-py uses setuptools_scm there must be an equivalent for poetry. I can take a look if you want.

Another simpler approach would be to patch this directly in the artifact built for system-tests here https://github.com/DataDog/datadog-lambda-python/blob/main/.github/workflows/build_layer.yml. It makes running local builds against system-tests less straight-forward and the versioning scheme less consistant but I guess it is the path of least resistance.

@florentinl
Copy link
Contributor

florentinl commented Jul 18, 2025

To clarify this point

my preliminary understanding is that those tests are using very limited resources and we are trying to make thing more directly accessible in the pipeline.

system-tests aren't really run with limited resource. My understanding from discussing with the system-tests team is that it is more of a problem of separation of concerns.

They work under the principle that system-tests shouldn't have to worry about how to build a library or how to derive the version depending on git metadata, these are the concerns of the library. system-tests only need to know where to fetch the library so they can use it.

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.

3 participants