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

feat: add support for python 3.12 #6626

Merged
merged 60 commits into from
Sep 14, 2023
Merged

feat: add support for python 3.12 #6626

merged 60 commits into from
Sep 14, 2023

Conversation

Yun-Kim
Copy link
Contributor

@Yun-Kim Yun-Kim commented Aug 9, 2023

Adds support for Python 3.12.

Checklist

  • Change(s) are motivated and described in the PR description.
  • Testing strategy is described if automated tests are not included in the PR.
  • Risk is outlined (performance impact, potential for breakage, maintainability, etc).
  • Change is maintainable (easy to change, telemetry, documentation).
  • Library release note guidelines are followed. If no release note is required, add label changelog/no-changelog.
  • Documentation is included (in-code, generated user docs, public corp docs).
  • Backport labels are set (if applicable)

Reviewer Checklist

  • Title is accurate.
  • No unnecessary changes are introduced.
  • Description motivates each change.
  • Avoids breaking API changes unless absolutely necessary.
  • Testing strategy adequately addresses listed risk(s).
  • Change is maintainable (easy to change, telemetry, documentation).
  • Release note makes sense to a user of the library.
  • Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment.
  • Backport labels are set in a manner that is consistent with the release branch maintenance policy

@Yun-Kim Yun-Kim changed the title Add python 3.12 testing, upgrade riot to 0.19.0 chore: add python 3.12 testing, upgrade riot to 0.19.0 Aug 9, 2023
@Yun-Kim Yun-Kim force-pushed the yunkim/add-py-312 branch from 988c7bc to d0fe3ba Compare August 10, 2023 18:58
@pr-commenter
Copy link

pr-commenter bot commented Aug 10, 2023

Benchmarks

Benchmark execution time: 2023-09-08 19:44:02

Comparing candidate commit 57317e6 in PR branch yunkim/add-py-312 with baseline commit b9c9efd in branch 2.x.

Found 0 performance improvements and 13 performance regressions! Performance is the same for 91 metrics, 0 unstable metrics.

scenario:flasksimple-baseline

  • 🟥 max_rss_usage [+765.509KB; +944.981KB] or [+2.300%; +2.840%]

scenario:flasksimple-debugger

  • 🟥 max_rss_usage [+803.321KB; +932.564KB] or [+2.413%; +2.801%]

scenario:flasksimple-iast-get

  • 🟥 max_rss_usage [+758.227KB; +906.387KB] or [+2.277%; +2.722%]

scenario:flasksimple-profiler

  • 🟥 max_rss_usage [+758.903KB; +906.531KB] or [+2.278%; +2.721%]

scenario:flasksimple-tracer

  • 🟥 execution_time [+306.933µs; +335.289µs] or [+5.151%; +5.627%]
  • 🟥 max_rss_usage [+683.501KB; +859.462KB] or [+2.054%; +2.583%]

scenario:otelspan-start-finish

  • 🟥 max_rss_usage [+758.492KB; +910.628KB] or [+2.648%; +3.179%]

scenario:sethttpmeta-collectipvariant_exists

  • 🟥 max_rss_usage [+763.658KB; +926.761KB] or [+2.741%; +3.326%]

scenario:sethttpmeta-obfuscation-send-querystring-disabled

  • 🟥 max_rss_usage [+1.044MB; +1.189MB] or [+3.752%; +4.273%]

scenario:sethttpmeta-obfuscation-worst-case-explicit-query

  • 🟥 max_rss_usage [+833.141KB; +975.243KB] or [+2.972%; +3.478%]

scenario:sethttpmeta-obfuscation-worst-case-implicit-query

  • 🟥 max_rss_usage [+1.033MB; +1.184MB] or [+3.711%; +4.252%]

scenario:sethttpmeta-useragentvariant_exists_1

  • 🟥 max_rss_usage [+798.287KB; +949.067KB] or [+2.868%; +3.410%]

scenario:span-start-finish

  • 🟥 max_rss_usage [+695.559KB; +866.246KB] or [+2.517%; +3.135%]

@majorgreys majorgreys force-pushed the 2.x-dev branch 2 times, most recently from b59b9a4 to 3052251 Compare August 24, 2023 19:15
@majorgreys majorgreys force-pushed the 2.x-dev branch 3 times, most recently from 49a3ea6 to 3cd8cd2 Compare August 25, 2023 16:34
@majorgreys majorgreys force-pushed the yunkim/add-py-312 branch 3 times, most recently from 2822659 to 60e25bf Compare August 25, 2023 17:53
@majorgreys majorgreys changed the base branch from 2.x-dev to 2.x August 29, 2023 14:34
@majorgreys majorgreys changed the base branch from 2.x to 2.x-dev August 30, 2023 16:29
@Yun-Kim Yun-Kim mentioned this pull request Aug 31, 2023
16 tasks
@Yun-Kim Yun-Kim force-pushed the yunkim/add-py-312 branch 2 times, most recently from 6848651 to 9869ad0 Compare August 31, 2023 20:01
Copy link
Contributor

@P403n1x87 P403n1x87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to make sure we don't remove tests that are still relevant

tests/debugging/py35/test_async.py Outdated Show resolved Hide resolved
@Yun-Kim Yun-Kim force-pushed the yunkim/add-py-312 branch 2 times, most recently from e85c602 to 919f560 Compare September 5, 2023 19:37
@Yun-Kim Yun-Kim requested a review from P403n1x87 September 5, 2023 20:30
@majorgreys majorgreys changed the title chore: add python 3.12 testing, upgrade riot to 0.19.0 feat: add support python 3.12 Sep 6, 2023
@majorgreys majorgreys changed the title feat: add support python 3.12 feat: add support for python 3.12 Sep 6, 2023
emmettbutler
emmettbutler previously approved these changes Sep 14, 2023
@emmettbutler
Copy link
Collaborator

@ZStriker19 @P403n1x87 @majorgreys @shatzi @Kyle-Verhoog This is actually ready for review now, with all tests passing. Please approve again at your convenience!

Copy link
Contributor

@P403n1x87 P403n1x87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of nits and questions. I think this needs a new rebase, otherwise LGTM!

ddtrace/internal/safety.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
Co-authored-by: Gabriele N. Tornetta <P403n1x87@users.noreply.github.com>
Co-authored-by: Gabriele N. Tornetta <P403n1x87@users.noreply.github.com>
@emmettbutler
Copy link
Collaborator

@P403n1x87 thanks for checking this out. what do you mean by "needs a new rebase"?

emmettbutler
emmettbutler previously approved these changes Sep 14, 2023
@P403n1x87
Copy link
Contributor

@P403n1x87 thanks for checking this out. what do you mean by "needs a new rebase"?

Oh sorry, I should have said that the target branch probably needs a new rebase as I think some changes are also available in 1.x at the moment, but still show up as diff in this PR (e.g. ddtrace/debugging/_expressions.py should have no diffs now that #6835 is merged in 1.x)

@emmettbutler
Copy link
Collaborator

@P403n1x87 ah, thanks for clarifying. do you know of any reason that needs to happen before this gets merged?

@emmettbutler emmettbutler merged commit b64f3e3 into 2.x Sep 14, 2023
@emmettbutler emmettbutler deleted the yunkim/add-py-312 branch September 14, 2023 15:45
majorgreys added a commit that referenced this pull request Sep 15, 2023
Adds support for Python 3.12.

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

---------

Co-authored-by: Tahir H. Butt <tahir.butt@datadoghq.com>
Co-authored-by: Gabriele N. Tornetta <gabriele.tornetta@datadoghq.com>
Co-authored-by: Emmett Butler <emmett.butler321@gmail.com>
Co-authored-by: ZStriker19 <zach.groves@datadoghq.com>
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
Co-authored-by: Emmett Butler <723615+emmettbutler@users.noreply.github.com>
majorgreys added a commit that referenced this pull request Sep 18, 2023
Adds support for Python 3.12.

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

---------

Co-authored-by: Tahir H. Butt <tahir.butt@datadoghq.com>
Co-authored-by: Gabriele N. Tornetta <gabriele.tornetta@datadoghq.com>
Co-authored-by: Emmett Butler <emmett.butler321@gmail.com>
Co-authored-by: ZStriker19 <zach.groves@datadoghq.com>
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
Co-authored-by: Emmett Butler <723615+emmettbutler@users.noreply.github.com>
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.

8 participants