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

Update packages and version to Python 3.12 #1530

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

Conversation

petechd
Copy link
Contributor

@petechd petechd commented Oct 8, 2024

What is the context of this PR?

This updates project's Python to version 3.12 and updates some of the dev libraries since the poetry lock required regeneration as well. As a result some new flake8 config and some test changes had to be made.

How to review

Run benchmark and test all the places where Python got updated.

Checklist

  • New static content marked up for translation
  • Newly defined schema content included in eq-translations repo

@petechd petechd added the python Pull requests that update Python code label Oct 8, 2024
@petechd petechd marked this pull request as ready for review October 8, 2024 12:16
@ons-eq-team
Copy link
Contributor

Benchmark Results

Percentile Averages:
50th: 84ms
90th: 153ms
95th: 259ms
99th: 842ms
99.9th: 1667ms
GETs (99th): 1249ms
POSTs (99th): 371ms

PDF: 2800ms
Session: 47000ms

Total Requests: 66,244
Total Failures: 0
Error Percentage: 0.0%

@berroar
Copy link
Contributor

berroar commented Oct 15, 2024

Needs a PR description though I accept the change is fairly obvious 😆

@berroar
Copy link
Contributor

berroar commented Oct 15, 2024

Benchmark ResultsPercentile Averages:50th: 84ms90th: 153ms95th: 259ms99th: 842ms99.9th: 1667msGETs (99th): 1249msPOSTs (99th): 371msPDF: 2800msSession: 47000msTotal Requests: 66,244Total Failures: 0Error Percentage: 0.0%

The stats here are not great, and although they have been up and down recently in benchmark i think it is worth running some more tests to assess whether there are some performance implications here.

@ons-eq-team
Copy link
Contributor

Benchmark Results

Percentile Averages:
50th: 83ms
90th: 146ms
95th: 236ms
99th: 931ms
99.9th: 1805ms
GETs (99th): 1450ms
POSTs (99th): 332ms

PDF: 3100ms
Session: 60000ms

Total Requests: 65,980
Total Failures: 1
Error Percentage: 0.0%

Copy link
Contributor

@Yuyuutsu Yuyuutsu left a comment

Choose a reason for hiding this comment

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

Do we need to do a Performance investigation in benchmark?

@ons-eq-team
Copy link
Contributor

Benchmark Results

Percentile Averages:
50th: 85ms
90th: 217ms
95th: 784ms
99th: 1300ms
99.9th: 2601ms
GETs (99th): 1765ms
POSTs (99th): 762ms

PDF: 4000ms
Session: 52000ms

Total Requests: 63,620
Total Failures: 2
Error Percentage: 0.0%

Copy link
Contributor

@Yuyuutsu Yuyuutsu left a comment

Choose a reason for hiding this comment

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

Changes look good, and benchmark results seem similar to the results you have run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants