Uh oh!
There was an error while loading. Please reload this page.
fir(test/__init__.py): Ignoring DeprecationWarning from "cassandra.io.asyncioreactor" - #1118
fir(test/__init__.py): Ignoring DeprecationWarning from "cassandra.io.asyncioreactor"#1118Orenef11 wants to merge 4 commits into
Conversation
Orenef11
commented
Nov 23, 2021
@absurdfarce Can you review this PR? |
absurdfarce
commented
Dec 10, 2021
@Orenef11 So I'm not sure I quite understand the intent here. Are the DeprecationWarnings coming from cassandra.io.asyncioreactor causing tests to fail? Or is the goal here merely to suppress any output that might result from these warnings? If we're actually seeing tests fail because of this problem I agree something needs to be done, but if we're only trying to limit some output logging that might be a different story. How much noise do these exceptions actually generate? And is there a case for fixing the root cause of the deprecations rather than suppressing the errors overall? I guess I'm just a bit nervous about the idea of explicitly suppressing errors reported by tests. :( Your comments mentioned that these errors were only visible on Python 3.8. I haven't tried a test run with that version of Cython yet... suppose I could just run through that exercise rather than being lazy and asking you all these questions. :) |
Orenef11
commented
Dec 12, 2021
@absurdfarce If you want, I can try to fix it for Python3.8. |
absurdfarce
commented
Dec 13, 2021
@Orenef11 If you'd like to try to resolve them I'm certainly fine with that. I'm trying to repro myself in order to see which deprecation messages are causing the most trouble.. but I don't think I'm doing the same thing you are: Is there a cleaner way to repro the DeprecationWarnings you're seeing? |
Orenef11
commented
Dec 13, 2021
@absurdfarce Run the test with |
absurdfarce
commented
Dec 13, 2021
Ah, thanks for that @Orenef11 , that was extremely useful! I see the errors you're referencing now. Please let me know if you run into any trouble implementing a fix. It looks like you'll have to fix cassandra.io.asyncioreactor and likely update the asynctest dependency as well (since some of the error messages are coming from there as well). We also need to make sure any changes will also run without issue for older Python runtimes, specifically 3.5 (the oldest version we currently support in the 3.x line) as well as 2.7. Thanks, and again please let me know if you need need any help! |
2283cc6 to
8974779Compare@absurdfarce |
Orenef11
commented
Dec 15, 2021
@absurdfarce |
Orenef11
commented
Dec 28, 2021
ping @absurdfarce |
| log = logging.getLogger(__name__) | ||
| is_min_python_version_3_5 = (sys.version_info.major, sys.version_info.micro) >= (3, 5) |
There was a problem hiding this comment.
Ill change it to system.version_info >= (3, 5)
absurdfarce
commented
Jan 4, 2022
@Orenef11 Apologies, everyone here has been out for holiday/end-of-year reasons. I'll try to take another look at this soon! Thanks for your patience! |
Orenef11
commented
Jan 4, 2022
NP, when you can please review it. |
8974779 to
19394c3Compare…yncio.coroutine * Replace all places that use old async node (`@asyncio.coroutine` and `yield from`) and use a new Python syntax (Only for Python v3.5+) * Save the old code to allow users with Python v2+ to run our tests. * requirements.txt: * Install `futures` package only for Python v2 * Use the latest `six` package version
…arning: invalid escape sequence \" warning
…Warning: invalid escape sequence \" warning
19394c3 to
39fcfbbCompareabsurdfarce
commented
Jan 25, 2022
@Orenef11 My sincerest apologies for the extended delay in getting back to you on this. I've been swamped with work for some of the other drivers supported by DataStax but (as will hopefully become apparent soon) there's also been a fair amount of work going on behind the scenes for this effort as well. I was a bit nervous about the extensive modifications to the test framework (like those outlined in this PR) just to remove some deprecation warnings. Perhaps even more importantly I couldn't shake the feeling that the deprecation warning really was trying to tell us something. In the not-too-distant future I'd like to catch the Python driver up with more modern Python runtimes and deprecation warnings such as these were indicating an area that would need to be addressed as part of any such effort. So I got to wondering... what's actually going on here and is it possible to just fix the underlying condition? The results of that investigation are documented in PYTHON-1290. My conclusion was that given the set of Python versions we're tasked with supporting now it does make sense to fix the underlying issue directly. I've opened another PR to do exactly that. At this point I think I'd like to close this PR in favor of the underlying fix. What do you think? I would certainly welcome your review of the other PR as well. |
Orenef11
commented
Jan 26, 2022
No problem, my goal is to improve the driver as much as I can :-) The current PR contains an additional commitment, can I open it in another PR? Last thing, is it possible to use |
absurdfarce
commented
Jan 26, 2022
Thanks (again) @Orenef11! I certainly appreciate all the effort you've put in so far! Just to make sure I have this right: #1120 is the other PR you mentioned, right? On the question of changing to pytest... where would you like to make this change? In the tox.ini setup for running tests (i.e. in favor of nose)? Or in the actual test classes themselves? |
its is other issue...
The short fix it change only the |
absurdfarce
commented
Feb 2, 2022
This issue has been addressed with the changes in #1119. Closing this one out. Re: changing how the unittests are run... I'm open to the idea of converting the process running the tests to pytest. I do not want to change the tests themselves to use the pytest API. unittest is part of the Python standard lib so I'm extremely reluctant to move away from that. |
@asyncio.coroutineandyield from) and use a new Python syntax (Only for Python v3.5+)futurespackage only for Python v2sexpackage version