-
Notifications
You must be signed in to change notification settings - Fork 73
[SYNPY-1690] python 3 14 #1273
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
base: gen-2381-table-typing
Are you sure you want to change the base?
[SYNPY-1690] python 3 14 #1273
Conversation
- Removed the use of `alru_cache` decorator in `get_upload_destination` function. - Introduced `store_async` method in the Synapse client for asynchronous entity storage. - Updated various methods to support asynchronous operations, including `getWiki_async`, `downloadTableColumns_async`, and `_storeWiki`. - Modified `wrap_async_to_sync` utility to raise an error when called within an existing async context in Python 3.14+. - Enhanced `sync` functions to provide asynchronous counterparts: `syncFromSynapse_async` and `syncToSynapse_async`. - Updated `Dataset` class to include asynchronous methods for adding and removing items. - Refactored `SchemaOrganization` and `JSONSchema` classes to support async operations. - Adjusted table component methods to accept a `synapse_client` parameter for better async handling.
…ods to async, update related methods, and ensure compatibility with async operations.
…enable tests to run
| from async_lru import alru_cache | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to drop the alru_cache here. I saw some errors when I was testing and removing this gave me a bit more stability.
The implication of this change is that if folks are uploading many files to a single location (ie: Folder), there would be a performance hit since each file needs to make a call to determine where it should be uploaded to.
cc: @linglp : This is also related to some discussion we were having while you were doing some upload testing. I know that we had seen some issues where this was hanging for some amount of time. This is just an fyi that I am planning to remove this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for letting me know, Bryan!
| if loop and sys.version_info >= (3, 14, 0): | ||
| raise RuntimeError( | ||
| f"Cannot use wrap_async_to_sync from within an existing async context in Python 3.14+, instead call the async function `{coroutine.__name__}` directly" | ||
| ) | ||
| elif loop: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am open to suggestions on making these messages more clear. This would tell the user what function they should call instead, however, it might not make the most sense to them if they aren't aware of asyncio.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I' wondering if an end-user would ever see this. This is something a dev should see during testing right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users would see this message if they are in an asyncio event loop and they call a synchrnous method. For example:
import asyncio
from pprint import pprint
from synapseclient import Synapse
from synapseclient.models import Project
syn = Synapse()
syn.login()
my_project = Project(name="My uniquely named project about Alzheimer's Disease").get()
pprint(my_project)
async def main():
project = Project(name="My uniquely named project about Alzheimer's Disease").get()
pprint(project)
asyncio.run(main())Gives this error on Python 3.14:
Project(id='syn53185532',
name="My uniquely named project about Alzheimer's Disease",
description=None,
etag='6da834f4-ea37-4bb1-bd27-034412b16169',
created_on='2023-12-22T20:39:54.812Z',
modified_on='2024-09-13T19:38:58.662Z',
created_by='3481671',
modified_by='3481671',
alias=None,
files=[],
folders=[],
tables=[],
entityviews=[],
submissionviews=[],
datasets=[],
datasetcollections=[],
materializedviews=[],
virtualtables=[],
annotations={},
parent_id='syn4489')
Traceback (most recent call last):
File "/home/ec2-user/BryansGreatWorkspaceSc/synapsePythonClient/junk3.py", line 17, in <module>
asyncio.run(main())
~~~~~~~~~~~^^^^^^^^
File "/home/ec2-user/.local/share/uv/python/cpython-3.14.0b2-linux-x86_64-gnu/lib/python3.14/asyncio/runners.py", line 204, in run
return runner.run(main)
~~~~~~~~~~^^^^^^
File "/home/ec2-user/.local/share/uv/python/cpython-3.14.0b2-linux-x86_64-gnu/lib/python3.14/asyncio/runners.py", line 127, in run
return self._loop.run_until_complete(task)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^
File "/home/ec2-user/.local/share/uv/python/cpython-3.14.0b2-linux-x86_64-gnu/lib/python3.14/asyncio/base_events.py", line 719, in run_until_complete
return future.result()
~~~~~~~~~~~~~^^
File "/home/ec2-user/BryansGreatWorkspaceSc/synapsePythonClient/junk3.py", line 14, in main
project = Project(name="My uniquely named project about Alzheimer's Disease").get()
File "/home/ec2-user/BryansGreatWorkspaceSc/synapsePythonClient/synapseclient/core/async_utils.py", line 70, in f
return self.fn(obj, *args, **kwds)
~~~~~~~^^^^^^^^^^^^^^^^^^^^
File "/home/ec2-user/BryansGreatWorkspaceSc/synapsePythonClient/synapseclient/core/async_utils.py", line 194, in newmethod
raise RuntimeError(
f"Cannot use wrap_async_to_sync from within an existing async context in Python 3.14+, instead call the async method `{async_method_name}` directly"
)
RuntimeError: Cannot use wrap_async_to_sync from within an existing async context in Python 3.14+, instead call the async method `get_async` directly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added more clarity to the messages in this commit:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| SYNAPSE_SCHEMA_URL = f"{Synapse().repoEndpoint}/schema/type/registered/" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrewelamb I changed this portion of the JSON Schema code.
The issue is that we need to get the repoEndpoint from the Synapse class instance that the user is using to make these calls. The way that this was set up is that it would ALWAYS be the production repoEndpoint that was being used, even if - for example, they were using the dev server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to make changes based on your feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These seem fine. LGTM!
…date pytest_asyncio fixture to pytest fixture.
…Python 3.14+ and adjust integration tests for async method usage
…uidelines and error handling



Problem:
Python 3.14 changes some of the implementation of asyncio (https://docs.python.org/3/whatsnew/3.14.html#asyncio). The patch notes are fairly innocent; however, we were using a (now deprecated) library to enable easier usage of SYNPY in Jupyter notebooks (https://github.com/erdewit/nest_asyncio), which no longer works and breaks HTTPCore when it is using connection pooling.
Why was it useful to use nest_asyncio before?
When a Jupyter notebook starts up, it runs all of its code within an asyncio event loop. This means:
asyncio.run(my_async_function())because the asyncio developers intentionally disallowed nested event loops (to avoid threading and deadlock issues)nest_asyncio monkey-patched asyncio to work around this limitation. It allowed us to call async functions from within synchronous wrapper functions, even when an event loop was already running. We used this wrapper function to detect whether we were in a notebook environment (with a running loop) or a regular Python script:
synapsePythonClient/synapseclient/core/async_utils.py
Lines 77 to 90 in a73bcc1
The wrapper would:
This pattern enabled our library to provide a synchronous API that worked seamlessly in both Jupyter notebooks and regular Python scripts, while using async operations internally.
What is the implication for this issue post Python 3.14?
This would work just fine when running it from your IDE or as a regular Python script in Python 3.14, but it will NOT work from a notebook:
This would work just fine when running it from your IDE or as a regular Python script in Python 3.14, but it will NOT work from a notebook (this behavior isn't new, it was always like this):
Solution:
Testing:
Integration/unit testing covers a large chunk of this as it found several areas that had issues
More comprehensive testing - Especially around behavior in Notebooks pre/post 3.14 Python is