Support Python 3.12

[nrbnlulu]

If you use 3.12 generics the action would fail.
i.e

class Foo[T]:
	bar: T

[ewjoachim]

Thank you for your report.
For the record, when you file reports to any project, please try and make it more informative, providing e.g. a build log or a copy of the stack trace or even just the error message.

With the amount of information you provided, I'm tempted to say "this is an issue with the coverage lib".

[nrbnlulu]

Coverage works fine. This happends due to the fact that the action runs on python3.11

Error: Critical error. This error possibly occurred because the permissions of the workflow are set incorrectly. You can see the correct setting of permissions here: https://github.com/py-cov-action/python-coverage-comment-action#basic-usage
Otherwise please look for open issues or open one in https://github.com/py-cov-action/python-coverage-comment-action/
Traceback (most recent call last):
  File "/workdir/coverage_comment/subprocess.py", line 22, in run
    return subprocess.run(
           ^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/subprocess.py", line 571, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '('coverage', 'json', '-o', '-')' returned non-zero exit status 1.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/workdir/coverage_comment/main.py", line 44, in main
    exit_code = action(
                ^^^^^^^
  File "/workdir/coverage_comment/main.py", line 88, in action
    return save_coverage_data_files(
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workdir/coverage_comment/main.py", line [33](https://github.com/tzevet5/Backend/actions/runs/8040223520/job/21958246919#step:8:34)8, in save_coverage_data_files
    raw_coverage_data, coverage = coverage_module.get_coverage_info(
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workdir/coverage_comment/coverage.py", line 99, in get_coverage_info
    subprocess.run("coverage", "json", "-o", "-", path=coverage_path)
  File "/workdir/coverage_comment/subprocess.py", line [36](https://github.com/tzevet5/Backend/actions/runs/8040223520/job/21958246919#step:8:37), in run
    raise SubProcessError("\n".join([exc.stderr, exc.stdout])) from exc
coverage_comment.subprocess.SubProcessError: 
Couldn't parse 'src/core/dto.py' as Python source: 'invalid syntax' at line 18

[ewjoachim]

This happens due to the fact that the action runs on python3.11

I don't agree with this. This happens because coverage json doesn't support being called by a Python version incompatible with the covered Python code (because it uses the stdlib of the Python version it runs with). A lot of existing tools allow decoupling runtime python and codebase python. Coverage does not.

This could be fixed in multiple ways:

• Python offering better forward/backward compatibility in ast parsing functions (this has been proposed, e.g. in PEP 606)
• Coverage using an external lib for ast parsing, and thus provide a way to change the python version for parsing the codebase
• Running the action with Python 3.12, indeed
• This action not using coverage at all
• ...

I'm not a huge fan of tying the Python version this lib runs on to the Python version of the code it analyses. Though that's probably the best solution in the short term (assuming it doesn't break on older python codebases).

I'll see what I can do. On a personal note, the tone of interactions in this ticket have been unpleasant so far. This isn't helping my will to prioritize spending my own personal time fixing the project I made so that it works for your case. You might want to reassess how limiting your communication to the bare minimum, including peremptory assertions and contradicting your interlocutor's hypothesis without supporting claims works for convincing people to do free work for you :) Still, you're probably not the only 3.12 user out there, so I'll consider doing it.

[nrbnlulu]

I'll see what I can do. On a personal note, the tone of interactions in this ticket have been unpleasant so far.

I am truly sorry that you were hearing that tone from me 😞.
And I really appreciate your work here don't get me wrong. I was just trying to be concrete.

As for the notes you left I generally agree though

I'm not a huge fan of tying the Python version this lib runs on to the Python version of the code it analyses.

Correct me if I'm wrong, but the workflow uses a docker image to do the work instead of setting up it's dependencies
using pip or something. If we were to use the current Python, would it work in any (supported) version 🤔 ?

[ewjoachim]

I am truly sorry that you were hearing that tone from me 😞.
And I really appreciate your work here don't get me wrong. I was just trying to be concrete.

Maybe I read too much on this. Communication is hard. I appreciate your acknowledgement, thank you. Back to the topic.

Correct me if I'm wrong, but the workflow uses a docker image to do the work instead of setting up it's dependencies
using pip or something.

Well we use both. Docker and poetry inside docker, I'm not sure to follow your train of thought

If we were to use the current Python, would it work in any (supported) version 🤔 ?

It all depends on coverage. I didn't think we needed the notion of "supported version" at all, I thought the coverage datafile was all we needed. Now I discover that if we run with py3.12, because of coverage, we need the code to be py3.12 compatible. While Python regularily adds new syntax features, they haven't made backwards incompatible changes lately (the last one being async and await becoming keywords, I guess). If they do so in the future, we'll need to choose between supporting a newer python syntax or an older python syntax, so we'll have a set of supported versions and it's a bit sad.

The more I think about it, the more replacing coverage with coberturapy (#347) sounds like a solution to many problems. Of course, that would mean a new major version, breaking compat ourselves etc.

[nrbnlulu]

Thanks for the quick fix!
I saw that you just updated the Python version eventually...

[ewjoachim]

Yep. It was still the right thing to do in this case, it's just that this issue made me realize that until we make bigger changes, I'll need to stay on top of python versions, I can't decide completely on my own when I want to upgrade while keeping compatibility for everyone.