chore: upgrade Python for all utils and refactor by taddes · Pull Request #2127 · mozilla-services/syncstorage-rs

taddes · 2026-03-13T19:36:51Z

Description

Testing result of workflow Python update

Issue(s)

Closes link.

chenba · 2026-03-19T13:48:31Z


+concurrency:
+  group: ${{ github.workflow }}-${{ github.head_ref || github.ref }}
+  cancel-in-progress: true


This and other similar edits are unrelated?

Yes, this was testing for the other PR. I've enabled it here to make quicker checks and shutting down the runs when making dep checks

chenba · 2026-03-19T13:58:20Z

+## Notes
+
+- The `--secret` value must match the `SYNC_MASTER_SECRET` configured in the running syncstorage-rs instance.
+- The generated token is scoped to the exact URI and method — a token generated for `GET /foo` is not valid for `PUT /foo`.


This sound right from a quick read but I'm no hawk auth pro. I also cannot tell if this is related to the Python upgrades.

Yeah, I wanted to add the docs in the tools/ dir to the ones in our main docs repo, and did a little revision across the board.

chenba · 2026-03-19T14:00:37Z

 The latter modifies the query string which changes the mac/nonce and
-potentially ts values (in the Hawk header).
-
+potentially its values (in the Hawk header).


The original text was correct. 'ts' as in timestamp.

chenba · 2026-03-19T14:18:41Z

 license = "Mozilla Public License Version 2.0"
 readme = "README.md"
-requires-python = ">=3.9"
+requires-python = ">=3.12"


I see 3.14 (from 3.12) so far except here. Why is that?

With Poetry, you don't have to explicitly specify the version your using, so long as the constraint, which is greater than 3.12 is sufficient. Sometimes Poetry will scream at you with certain dependencies (like some we use that are outdated) if the version specifier is too high.

chenba · 2026-03-19T14:27:14Z

+    """Same input always produces the same hash."""
+    assert metrics_hash(make_args(), "abc") == metrics_hash(make_args(), "abc")
+
+


Better than no tests I guess. But if metrics_hash returns the same hex string every time the two tests above would still pass.

...and the two tests below as well.

chenba · 2026-03-19T14:32:12Z

+    result = metrics_hash(args, "value")
+    assert isinstance(result, str)
+    assert args.hmac_key == b"foo"
+


If we really want to test metrics_hash then we need to compare its output against known values.

Good point, I then pre-computed a value using hmac and hashlib, put it in the test and compare against it, so that's better 👍

basically:

import hmac import hashlib res = hmac.new(b'foo', b'value', hashlib.sha256).hexdigest())

chenba · 2026-03-19T17:19:41Z

+
+
+def test_create_token_expires_in_future() -> None:
+    """The expiry timestamp is strictly after the current time."""


Not so sure about "strictly". create_token doesn't even validate the duration argument.

chenba · 2026-03-19T17:30:39Z

+    assert decoded["uid"] == args.uid
+    assert decoded["node"] == args.node
+    assert decoded["fxa_uid"] == args.fxa_uid
+    assert decoded["fxa_kid"] == args.fxa_kid


Why not test the expiration this way too?

chenba · 2026-03-19T18:49:17Z


-    def test_invalid_expiry_mode(self):
+    def test_invalid_expiry_mode(self) -> None:
+        """An unrecognised expiry mode raises an exception."""


How very British.

I can't help but honour my Canadian heritage 🇨🇦 @chenba

pjenvey · 2026-03-20T18:17:59Z

 env:
  RUST_VERSION: "1.91" # RUST_VER
-  PYTHON_VERSION: "3.12" # PY_VER
+  PYTHON_VERSION: "3.14" # PY_VER


I'll note again -- AFAICT this and RUST_VERSION aren't actually used anywhere

Aha, good point, it's a hangover from the original run of setting up the actions. Only the rust and python setup actions use this. I can flag this on my current workflow refactor and make sure to take it out, since these will move into the root workflow.

That or I can yank them out here, since they're not doing anything 👍

chenba · 2026-03-20T21:24:31Z

+ruff = "^0.15.6"
+black = "^26.3.1"
+bandit = "^1.9.4"
+isort = "^8.0.1"


I'm a Python novice so maybe I simply don't understand how any of this works...but I don't think some of these deps are used? How and where is isort invoked?

Ah yes, I'm probs the only one using these and running them with poetry... I had meant to add makefile targets to use these utilities. I will make sure to do that in another PR and include it as a part of our Python checks workflows since yeah, not doing us much good. 😹

In short, ruff does most of the heavy lifting with format checks and linting, but isort makes the imports PEP8 compliant, bandit checks for vulnerabilities, pydocstyle provides utilities for code documentation, black is a formatter, and mypy gives us type safe-ish Python with proper annotation checks and validation.

pjenvey · 2026-03-24T18:52:24Z

+from __future__ import annotations
+


I think these are no longer needed on Python 3.14, which we're moving to?

Suggested change

from __future__ import annotations

Yeah good point then, looked it up to be sure and it's the default to support the PEP 649 lazy annotation evaluation. Thx for catching 👍

…it works for prs and branches

…e necessity

taddes force-pushed the chore/update-python-STOR-485 branch 2 times, most recently from d502057 to fb859fa Compare March 16, 2026 19:28

taddes changed the title ~~WIP: testing python update in workflows only~~ chore: upgrade Python for all utils and refactor Mar 16, 2026

taddes force-pushed the chore/update-python-STOR-485 branch 2 times, most recently from d813312 to b8229ba Compare March 18, 2026 15:20

taddes self-assigned this Mar 18, 2026

taddes marked this pull request as ready for review March 18, 2026 18:36

chenba reviewed Mar 19, 2026

View reviewed changes

taddes force-pushed the chore/update-python-STOR-485 branch from a495a27 to 896de4b Compare March 19, 2026 21:54

taddes requested a review from pjenvey March 20, 2026 15:35

pjenvey reviewed Mar 20, 2026

View reviewed changes

taddes force-pushed the chore/update-python-STOR-485 branch from c79ec34 to d14393d Compare March 20, 2026 19:09

taddes requested review from chenba and pjenvey March 20, 2026 19:09

chenba reviewed Mar 20, 2026

View reviewed changes

taddes force-pushed the chore/update-python-STOR-485 branch from d14393d to 5132b98 Compare March 23, 2026 17:28

pjenvey reviewed Mar 24, 2026

View reviewed changes

taddes force-pushed the chore/update-python-STOR-485 branch from abe18ad to f7221ea Compare March 24, 2026 19:07

taddes added 4 commits March 24, 2026 15:07

experiment with workflow python update only

637ef0b

hawk util updates, test script, deps

92ca159

postgres utils update

896353c

mypy updates postgreS

089c62e

taddes added 17 commits March 24, 2026 15:07

test concurrency rules for workflows

846b1a7

spanner utils, conftest, write_batch

483c9cc

update github concurrency group refs to support head_ref and ref, so …

a2c9ffe

…it works for prs and branches

update root poetry

db0174f

update python version for setup-python action

4f83dea

have to prefix workflow to group

ffe73fe

test cancel actions and new hawk token docs

bad9ce8

integration test updates

5fb0296

update spanner

cec7a45

tokenserver loadtests

cb35d44

tokenserver deps

94c302e

sync load tests

f290682

attempted fix to inject 3.14 instead of using apt-get

2b33bea

revert attempts to inject python image, revisit due to pyramid upgrad…

7d457e7

…e necessity

hawk token test rev

fbec254

rmv useless version deps from db workflows

771c4a3

rmv __future__ as not needed after PEP 649

f7221ea

pjenvey approved these changes Mar 24, 2026

View reviewed changes

taddes merged commit 6b09e99 into master Mar 24, 2026
31 checks passed

taddes deleted the chore/update-python-STOR-485 branch March 24, 2026 20:23

		"""Same input always produces the same hash."""
		assert metrics_hash(make_args(), "abc") == metrics_hash(make_args(), "abc")



		def test_create_token_expires_in_future() -> None:
		"""The expiry timestamp is strictly after the current time."""

Conversation

taddes commented Mar 13, 2026

Description

Issue(s)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taddes Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

taddes Mar 20, 2026 •

edited

Loading