chore: remove eol python 3.7

[devkral]

Remove python 3.7 and add python 3.11 to remaining test matrices

This PR also updates the testrunner docker image to python 3.8 and fully qualifies the source

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Documentation

Checklist

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage). (maximal damage: broken tests)

[codecov]

Codecov Report

Merging #2907 (1b36738) into main (313290c) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2907      +/-   ##
==========================================
- Coverage   96.53%   96.49%   -0.04%     
==========================================
  Files         467      467              
  Lines       28910    28905       -5     
  Branches     3548     3545       -3     
==========================================
- Hits        27908    27892      -16     
- Misses        821      827       +6     
- Partials      181      186       +5     

[botberry]

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:

---

This release removes support for Python 3.7 as its end of life
was on 27 Jun 2023.

This will allow us to reduce the number of CI jobs we have,
and potentially use newer features of Python. ⚡

---

Here's the preview release card for twitter:

Here's the tweet text:

🆕 Release (next) is out! Thanks to Alexander for the PR 👏

Get it here 👉 https://github.com/strawberry-graphql/strawberry/releases/tag/(next)

[devkral]

this PR also updates the dependencies (required because of python update)

[kristjanvalur]

If 3.7 is dropped, then the special handling of asyncio.CancelledError can be removed, since it becomes a direct child of BaseException with 3.8

[devkral]

as far as I can see, asyncio.CancelledError are not special handled (only for cleanup they are suppressed but for a good reason)

[kristjanvalur]

as far as I can see, asyncio.CancelledError are not special handled (only for cleanup they are suppressed but for a good reason)

Code like this:

        try:
            ...
        except asyncio.CancelledError:
            raise
        except Exception as error:
            await self.handle_task_exception(error)  # pragma: no cover
        

is written for 3.7 compatibility, where asyncio.CancelledError is a subclass of Exeption yet should not be handled but re-raised.
In 3.8 it is a direct BaseException and the above clause can be omitted (because there is no except BaseException anywhere in sight)

It's fine to leave it in, just that we should remember to remove it at some later point since it is redundant for 3.8 and above.

[devkral]

is something like

--- a/strawberry/subscriptions/protocols/graphql_transport_ws/handlers.py
+++ b/strawberry/subscriptions/protocols/graphql_transport_ws/handlers.py
@@ -108,8 +108,6 @@ class BaseGraphQLTransportWSHandler(ABC):
             self.connection_timed_out = True
             reason = "Connection initialisation timeout"
             await self.close(code=4408, reason=reason)
-        except asyncio.CancelledError:
-            raise
         except Exception as error:
             await self.handle_task_exception(error)  # pragma: no cover
         finally:
@@ -118,7 +116,10 @@ class BaseGraphQLTransportWSHandler(ABC):
             self.completed_tasks.append(task)
 
     async def handle_task_exception(self, error: Exception) -> None:
-        self.task_logger.exception("Exception in worker task", exc_info=error)
+        if isinstance(error, asyncio.CancelledError):
+            self.task_logger.exception("Worker task cancelled", exc_info=error)
+        else
+            self.task_logger.exception("Exception in worker task", exc_info=error)
 
     async def handle_message(self, message: dict) -> None:
         handler: Callable

right?
Handling CancelledErrors in handle_task_exception is quite a change

[kristjanvalur]

Just the first part. In 3.8 CancelledError won't be caught by "except Exception" because it isn't an "Exception"

[kristjanvalur]

I think I may have confused you with my talk about "CancelledError". Basically, we never want to catch it.
Previously, for 3.7, to ensure that we didn't, we had to add in except asyncio.CancelledError: raise clauses, in order to avoid catching it. for 3.8 and above, this is no longer necessary, as long as you are not catching BaseException (which you should almost never do).

So, the only thing required, is to simply drop the except asyncio.CancelledError: raise clauses

[devkral]

I think I may have confused you with my talk about "CancelledError". Basically, we never want to catch it. Previously, for 3.7, to ensure that we didn't, we had to add in except asyncio.CancelledError: raise clauses, in order to avoid catching it. for 3.8 and above, this is no longer necessary, as long as you are not catching BaseException (which you should almost never do).

So, the only thing required, is to simply drop the except asyncio.CancelledError: raise clauses

in some new python versions asyncio.CancelledError inherits from Exception, so we need it

[devkral]

cleaned up and rebased.

Also add the updated dependencies in the release.md

[kristjanvalur]

in some new python versions asyncio.CancelledError inherits from Exception, so we need it

I think you will find that this is incorrect. The documentation clearly states: "Changed in version 3.8: CancelledError is now a subclass of BaseException."

This special handling of CancelledError is not required in 3.8 and beyond, indeed, this is the reason it was changed in 3.8 because people were accidentally catching it and code like this was needed all over the place. No more.

[devkral]

technically this stanca would be also right, if they inherit from Exception, so it is implementation specific

[kristjanvalur]

technically this stanca would be also right, if they inherit from Exception, so it is implementation specific

No it isn't. Yes, you could deliberately mis-interpret it, but it indicates a change from the 3.7 behaviour. It is not implementation specific. I don't quite understand why you are being so stubborn about this, it is a well known fact of Python asyncio programming. If you like, I can have it fixed in the documentation.

This is from the 3.8 release notes:

.. bpo: 32528
.. date: 2019-05-23-17-37-22
.. nonce: sGnkcl
.. section: Library

Make asyncio.CancelledError a BaseException.

This will address the common mistake many asyncio users make: an "except
Exception" clause breaking Tasks cancellation.

In addition to this change, we stop inheriting asyncio.TimeoutError and
asyncio.InvalidStateError from their concurrent.futures.* counterparts.
There's no point for these exceptions to share the inheritance chain.

and from the "what's new" part:

* The exception :class:`asyncio.CancelledError` now inherits from
  :class:`BaseException` rather than :class:`Exception` and no longer inherits
  from :class:`concurrent.futures.CancelledError`.
  (Contributed by Yury Selivanov in :issue:`32528`.)

[devkral]

Thing is: vscode says something different. Maybe the stubs are wrong.

If you say it works, then I give it a try.

I just don't want to break anything as I have not so much time with two children. I just wanted to make a short PR, updating the code and now it takes much more time than anticipated.

Optimizing code is something which can be done afterwards so please forgive my stubbornness

[devkral]

this too?

        except asyncio.CancelledError:
            # CancelledErrors are expected during task cleanup.
            pass

EDIT: in case it is superfluous, can please someone else test it and make another PR. I have currently not the time.

[kristjanvalur]

It is fine, I can clean this all up later, I just wanted to mention this in the PR since this was about deprecating 3.7 and I have been wanting to get this out of the way for a long time.

Fwiw, I made a change to the docs, and here you have it from the horse's mouth:
python/cpython#106453 (review)

[devkral]

thank you.

[maximyurevich]

Also uvicorn can be updated with editing this PR

[patrick91]

@devkral can you rebase this? 😊

[devkral]

done

[devkral]

I hope it is ok to add an editorconfig as my defaults always wreck the yaml formatting.

[codspeed-hq]

CodSpeed Performance Report

Merging #2907 will not alter performance

_{Comparing devkral:chore/no_py37 (1b36738) with main (313290c)}

Summary

✅ 12 untouched benchmarks

[devkral]

there were some new refs to python 3.7 which had to be removed (tests/pyright).

Hope we can end the never ending PR soon ;)

[patrick91]

my only concern with this is that we support a version of django that still supports 3.7, I'm probably overthinking this though :D

I'll put this on my list of things to review this week :)

[patrick91]

Let's do this 😊