ci: adjust minimum Python version in broad charm compatibility tests

[tonyandrewmeyer]

A large number of our broad charm compatibility tests fail because the charm declares that it works with Python 3.8. That conflicts with us pushing the latest ops 3.x into the dependencies.

Generally, if we've made a change to ops[...] that breaks charms, that will be across Python versions, and we're more concerned with 3.10+ anyway (and in main, we're only supporting 3.10+). We could have some compatibility tests that check the latest 2.x branch for charms that expect 3.8, but that doesn't seem as useful.

This PR adjusts the dependency patching to also handle bumping the Python version to 3.10. It reduces the number of failures to 29, some of which are only for static checks (which I haven't worked on in this PR at all).

Since the changes are more complex now, they are moved out of the action file (YAML+bash) and into a helper (Python) script. Primarily, we do the work through tox, which I found to be the most consistent way to handle things across packaging tools (particularly given that we are using wheels now rather than git references).

The script also adds support for tox.toml, as some charms use that rather than tox.ini.

We also set PIP_IGNORE_REQUIRES_PYTHON to 1.

There is also a specific fix for wordpress-k8s, which can't handle 3.12 because of missing wheels. We could perhaps skip this charm instead, but it seems like a common example so one we should support (and it seems like too much of a job to add the missing 3.12 support).

Drive-by to fix mysql-router-operators which is now a monorepo (and some assorted changes that come through re-running the update script).

[dimaqq]

+1 on skipping wordpress

[dimaqq]

I'm generally on board with this.

[dimaqq]

Should this only be done if there's no py310 env yet?

[dimaqq]

This is pretty complex.
That being said I can't necessarily think of anything else, apart from maybe python -m pip ...

[dimaqq]

Would it be better to match pyproject.toml and relock instead?

[tonyandrewmeyer]

I started out with separate solutions that focused on pyproject.toml. The problem is that there are so many ways that the dependencies can be present, sometimes multiple within a charm:

• pyproject.toml, uv
• pyproject.toml, poetry
• requirements.txt, pip
• dependencies listed in tox

The only thing we definitely have is tox, because we don't support running tests with anything else (make, just) at the moment. So it ended up being cleanest to just lean into using tox, even though it's not what you'd typically do for really managing these dependencies.

[dimaqq]

This script will be hard to debug when one day something breaks.

I don't have a good idea how to make it better, but I feel I have to call this out.

[tonyandrewmeyer]

I don't disagree, although I would say that the previous YAML file approach was also not particularly simple to debug either. I think it is made easier by being a standalone script, and also by being Python rather than a bunch of shell commands. However, that's countered by it doing much more than before :(

[james-garner-canonical]

I like the overall approach of switching to a Python script for this more complicated logic. Going via tox to avoid too much special casing for all the different ecosystem tools seems like a great idea.

I do find the script hard to digest, I think primarily because there's a lot of long nesting conditionals. I think it would be a great idea to refactor a little bit to use more helpers and early returns / continues to make the high-level logic easier to follow. I've left some suggestions along these lines as I've scanned through.

I'm also wondering if unit tests for the factored out units might be nice to have, but they're not a blocker for me. I do wonder how we'll keep track of what's working as intended, but tracking what's passing and failing across multiple runs could be a separate project altogether.

[james-garner-canonical]

I'm getting my head around this new approach. My biggest question is: is patching commands_post actually doing what we want, or should we be patching commands_pre?

I've made a number of suggestions that I think will improve the script's digestibility as I go through the logic. The logic itself seems solid, though it's difficult to verify.

[james-garner-canonical]

Suggested change

	new_deps_lines = [
	line.strip()
	for line in deps.split('\n')
	if line.strip() and not _is_ops_dependency_line(line.strip())
	]
	original_count = len([ln for ln in deps.split('\n') if ln.strip()])
	if len(new_deps_lines) == original_count:
	deps_lines = [l for line in deps.splitlines() if (l := line.strip())]
	new_deps_lines = [l for l in deps_lines if not _is_ops_dependency_line(l)]
	if deps_lines == new_deps_lines:

[james-garner-canonical]

Suggested change

	if config.has_option(section, 'basepython'):
	basepython = config.get(section, 'basepython')
	if basepython in ('python3.8', 'python3.9'):
	config.set(section, 'basepython', 'python3.10')
	modified = True
	if config.get(section, 'basepython', fallback=None) in ('python3.8', 'python3.9'):
	config.set(section, 'basepython', 'python3.10')
	modified = True

[james-garner-canonical]

WDYT about using a library for this?

For example, we could do something like this (just need to install uv in the workflow):

#!/usr/bin/env -S uv run --script --no-project

# /// script
# requires-python = ">=3.12"
# dependencies = [
#     "tomli-w~=1.2.0",
# ]
# ///

and then

original = tomllib.loads(tox_config.read_text())
content = copy.deepcopy(original)
content['basepython'] = 'python3.10'
# ...
if content != original:
    tox_config.write_text(tomli_w.dumps(content))

[james-garner-canonical]

Are these redundant with the previous line?

[james-garner-canonical]

Would this do the job?

Suggested change

	new_envlist = re.sub(r'\bpy38\b,?\s*', '', new_envlist)
	new_envlist = re.sub(r'\bpy39\b,?\s*', '', new_envlist)
	new_envlist = re.sub(r'\{py38\},?\s*', '', new_envlist)
	new_envlist = re.sub(r'\{py39\},?\s*', '', new_envlist)
	# Clean up any trailing commas or whitespace
	new_envlist = re.sub(r',\s*$', '', new_envlist)
	new_envlist = re.sub(r',\s*\n', '\n', new_envlist)
	envs = [e for e in envlist.split(',') if not re.search(r'py3[89]', e)]
	new_envlist = ','.join(envs)

[james-garner-canonical]

Shouldn't we just crash if the charm has a tox.toml that isn't valid TOML?

Suggested change

	try:
	with open(tox_config, 'rb') as f:
	data = tomllib.load(f)
	except tomllib.TOMLDecodeError:
	print(f' ⚠ Warning: Could not parse {tox_config.name}')
	return False
	data = tomllib.loads(tox_config.read_text())

[james-garner-canonical]

Suggested change

	req_files = list(args.charm_root.glob('*requirements*.txt'))
	if req_files:
	if list(args.charm_root.glob('*requirements*.txt')):

[james-garner-canonical]

How about moving updated = False to the else branch?

[james-garner-canonical]

What we do in each branch looks identical to me.

[james-garner-canonical]

I don't understand this approach. Won't commands_post install our wheels after the commands (tests) have run? Should we be updating commands_pre instead?