Skip to content
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

Switch to log.warning instead of log.warn #1648

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Jenselme
Copy link
Contributor

log.warn is removed in Python 3.13

Close #1098

log.warn is removed in Python 3.13

Close crossbario#1098
@Jenselme Jenselme force-pushed the logger-warn-deprecated branch from 966a86c to e37d449 Compare August 22, 2024 07:32
@Jenselme
Copy link
Contributor Author

From what I understand of the error, I may be completely wrong on how to solve this: I seem to have modified calls to twited Logger class which does use warn. So the issue about warn being deprecated may come from a dependency and not this packages directly (nor txaio).

Testing this properly is made harder by the fact that pyunormalize cannot be installed on Python 3.13 yet

Any insights no this?

@oberstet
Copy link
Contributor

thanks for taking care of such things .. pls lemme comment ..

log.warn is removed in Python 3.13

if so, then

  • the CI in Autobahn should first be upgraded to 3.13 in an independent PR
  • this should then run red (fail), because the function is removed

once that is done, this PR can continue ..


right now, 3.13 is not yet released

grafik


currently, the CI uses CPython 3.11 and PyPy 3.9

python-version: ['3.9', '3.11', 'pypy-3.9']

and PyPy is now at 3.10

https://pypy.org/download.html

for the CI upgrade PR, we should first define the exact version goals for CPython and for PyPy ..

@Jenselme
Copy link
Contributor Author

Sounds good to me. I started looking into it because fedora is starting to switch to 3.13 in its dev version and it seemed easy to do (sadly I was proven wrong).

But, yes, we can wait for Python 3.13 to be officially release and migrate the CI first.

@oberstet
Copy link
Contributor

oberstet commented Aug 23, 2024

But, yes, we can wait for Python 3.13 to be officially release and migrate the CI first.

I'm confident this ordering will support Autobahn's goal of improving robustness & production stability.

if there is an issue in Autobahn triggered by a dependency upgrade, the issue should first be proven to actually exist and be valid

this requires the CI to first not touch any code, but only upgrade the dependency, and then demoonstrate at least one failing test (which previously worked).

if no such test currently exists which can demonstrate the issue, a new test case should be added

once the issue is confirmed (by a test that transitioned from GREEN to RED), the issue can be fixed in another PR

this allows to exactly follow the sequence of events with proof at each step, and the option for relatively easy rollback should something go wrong. as there will more often than not in reality;)

like ripples in a subsequent, dependent project like Crossbar.io (which obviously uses Autobahn a lot) ..

@yan12125
Copy link
Contributor

yan12125 commented Nov 3, 2024

Python 3.13 is out. Time to move on?

@oberstet
Copy link
Contributor

oberstet commented Nov 3, 2024

@yan12125 wau. you are quick;) I also saw that flying by. so yes: +1!

maybe worth: I think it would be good to first (and only that) switch from 3.11 to 3.13 everywhere in CI etc, e.g.

- name: Set up Python 3.11

and once that runs, only then move on to the log issue (the original itch of this issue), and this PR

in other words, what I am saying is: it might be worth doing the 3.11 to 3.13 switch (and only that) in a (new) separate PR

this latter (new) PR should be all green immediately. in theory. I don't believe it. sadly.

then this PR can simply catch up with / merge main to have 3.13

it will then run as is right away, and we'll see what is yet missing then ...

sounds like a good / workable plan?

@yan12125
Copy link
Contributor

yan12125 commented Nov 3, 2024

Sounds good. I can work on 3.11 -> 3.13 switch if you wish.

@yan12125
Copy link
Contributor

yan12125 commented Nov 4, 2024

Sounds good. I can work on 3.11 -> 3.13 switch if you wish.

I started a work-in-progress: https://github.com/yan12125/autobahn-python/commits/python-3.13/

@yan12125
Copy link
Contributor

yan12125 commented Nov 5, 2024

The linked issue #1098 mentions:

FYI, logger.warn is removed in Python 3.13: https://docs.python.org/3.13/whatsnew/3.13.html#logging

That page on Python official website actually places logging.warn in the section "Pending Removal in Future Versions", so that function is still in Python 3.13. Also, Python 3.13 unit tests passed without changes in this pull request: https://github.com/yan12125/autobahn-python/actions/runs/11666669688.

@Jenselme could you check how to make deprecation warnings visible in unit tests?

@Jenselme
Copy link
Contributor Author

@yan12125 I’m already seeing the warnings:

.tox/py3.11-asyncio/lib64/python3.11/site-packages/websockets/legacy/__init__.py:6
  /home/jenselme/Projects/_Clones/autobahn-python/.tox/py3.11-asyncio/lib64/python3.11/site-packages/websockets/legacy/__init__.py:6: DeprecationWarning: websockets.legacy is deprecated; see https://websockets.readthedocs.io/en/stable/howto/upgrade.html for upgrade instructions
    warnings.warn(  # deprecated in 14.0

It’s still not easy to spot where the issue comes from though.

@yan12125
Copy link
Contributor

@yan12125 I’m already seeing the warnings:

.tox/py3.11-asyncio/lib64/python3.11/site-packages/websockets/legacy/__init__.py:6
  /home/jenselme/Projects/_Clones/autobahn-python/.tox/py3.11-asyncio/lib64/python3.11/site-packages/websockets/legacy/__init__.py:6: DeprecationWarning: websockets.legacy is deprecated; see https://websockets.readthedocs.io/en/stable/howto/upgrade.html for upgrade instructions
    warnings.warn(  # deprecated in 14.0

Thanks, but the warning is about warnings.warn, not log.warning .

It’s still not easy to spot where the issue comes from though.

You may check it by running export PYTHONWARNINGS=error first.

@Jenselme
Copy link
Contributor Author

Currently, the test suite fails to run with Python 3.13 because of jnwatson/py-lmdb#362 Will try to monitor this issue and come back to this once it’s solved.

@oberstet
Copy link
Contributor

ah, right. thanks for digging! the PR jnwatson/py-lmdb#368 claims to solve that, so let's wait and see, yeah.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

logger.warn is removed in Python 3.13
3 participants