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

Change get_current_traceback to DebugTraceback #1993

Closed
wants to merge 20 commits into from

Conversation

deepyaman
Copy link

@deepyaman deepyaman commented Mar 28, 2022

Closes #1992

It's not an option to change get_current_traceback to DebugTraceback entirely because Werkzeug==2.1.0 requires Python 3.7, but we can backport a minimal DebugTraceback to earlier versions of Werkzeug.

Contributor Checklist

  • I have broken down my PR scope into the following TODO tasks
    • task 1
    • task 2
  • I have run the tests locally and they passed. (refer to testing section in contributing)
  • I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

optionals

  • I have added entry in the CHANGELOG.md
  • If this PR needs a follow-up in dash docs, community thread, I have mentioned the relevant URLS as follows
    • this GitHub #PR number updates the dash docs
    • here is the show and tell thread in Plotly Dash community

@deepyaman deepyaman changed the title Update dash.py Change get_current_traceback to DebugTraceback Mar 28, 2022
@deepyaman
Copy link
Author

deepyaman commented Mar 28, 2022

Closing this PR, as upgrading to Werkzeug 2.1.0 would mean dropping support for Python 3.6, which is a much bigger decision.

@deepyaman deepyaman closed this Mar 28, 2022
@deepyaman deepyaman reopened this Mar 28, 2022
@deepyaman deepyaman changed the title Change get_current_traceback to DebugTraceback Pin Flask<2.1.0 to retain Python 3.6 compatibility Mar 28, 2022
@deepyaman deepyaman changed the title Pin Flask<2.1.0 to retain Python 3.6 compatibility Pin Flask<2.1.0 and Werkzeug<2.1.0 to retain Python 3.6 compatibility Mar 28, 2022
@deepyaman deepyaman marked this pull request as ready for review March 28, 2022 19:54
@deepyaman deepyaman requested a review from alexcjohnson as a code owner March 28, 2022 19:54
@deepyaman deepyaman changed the title Pin Flask<2.1.0 and Werkzeug<2.1.0 to retain Python 3.6 compatibility Pin Flask<2.1.0, Werkzeug<2.1.0, and click<8.1.0 to retain Python 3.6 compatibility Mar 28, 2022
Copy link

@flying-sheep flying-sheep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version upper bounds are no good approach for library projects: https://iscinumpy.dev/post/bound-version-constraints/

You should instead write conditional code handling all flask/werkzeug versions or (if that’s too hard) drop Python 3.6 support

@deepyaman
Copy link
Author

Version upper bounds are no good approach for library projects: https://iscinumpy.dev/post/bound-version-constraints/

You should instead write conditional code handling all flask/werkzeug versions or (if that’s too hard) drop Python 3.6 support

@flying-sheep I can do either. I don't see much examples of conditional Python version handling within Dash (only

if sys.version_info[0] >= 3:
outside of tests, but even that's for a component generator), but I've seen plenty of it in other libraries. I would personally also be fine to drop 3.6, but don't think that's my call.

Are you one of the maintainers of Dash? Just want to makes sure the change is aligned with whoever will be making the final call.

@flying-sheep
Copy link

flying-sheep commented Mar 29, 2022

I’m not, sorry if I gave the impression. I think you can see that by the color of my review icon: mine is gray, so it’s just a “suggested change” and not a “requested change”.

I’m just speaking from my history of dealing with compatibility problems and bugs in python libraries. That blog post I linked really nails it IMHO, and I think any library maintainer should read it. Upper version bounds just cause too many problems for too many people.

@deepyaman
Copy link
Author

I’m not, sorry if I gave the impression. I think you can see that by the color of my review icon: mine is gray, so it’s just a “suggested change” and not a “requested change”.

I’m just speaking from my history of dealing with compatibility problems and bugs in python libraries. That blog post I linked really nails it IMHO, and I think any library maintainer should read it. Upper version bounds just cause too many problems for too many people.

No worries, and thanks for the article link--I've been going through it and sharing it to my colleagues, since there's a lot of great stuff with examples in there!!

@T4rk1n
Copy link
Contributor

T4rk1n commented Mar 29, 2022

I think we can drop support for Python 3.6 in later releases, it's been end of life for a few months now and more libraries will stop working as they drop support. What do you think @alexcjohnson ?

@alexcjohnson
Copy link
Collaborator

Py3.6 still accounts for about 10% of our downloads so I don't think it's time to drop it yet. 3.7 is still the most used though it's about to lose that title to 3.8 finally.

@deepyaman deepyaman changed the title Pin Flask<2.1.0, Werkzeug<2.1.0, and click<8.1.0 to retain Python 3.6 compatibility Change get_current_traceback to DebugTraceback Mar 29, 2022
aitmlouk added a commit to aitmlouk/stackn that referenced this pull request Mar 29, 2022
Fix for ImportError: cannot import name 'get_current_traceback' from 'werkzeug.debug.tbtools' conflict with dash for studio

celery-beat      |     from .dash_demo import app
celery-beat      |   File "/app/monitor/dash_demo.py", line 8, in <module>
celery-beat      |     import dash
celery-beat      |   File "/usr/local/lib/python3.8/site-packages/dash/__init__.py", line 5, in <module>
celery-beat      |     from .dash import Dash, no_update  # noqa: F401,E402
celery-beat      |   File "/usr/local/lib/python3.8/site-packages/dash/dash.py", line 22, in <module>
celery-beat      |     from werkzeug.debug.tbtools import get_current_traceback
celery-beat      | ImportError: cannot import name 'get_current_traceback' from 'werkzeug.debug.tbtools' (/usr/local/lib/python3.8/site-packages/werkzeug/debug/tbtools.py)

Ref: plotly/dash#1993
aitmlouk added a commit to aitmlouk/stackn that referenced this pull request Mar 29, 2022
Fix for ImportError: cannot import name 'get_current_traceback' from 'werkzeug.debug.tbtools' conflict with dash for studio

celery-beat      |     from .dash_demo import app
celery-beat      |   File "/app/monitor/dash_demo.py", line 8, in <module>
celery-beat      |     import dash
celery-beat      |   File "/usr/local/lib/python3.8/site-packages/dash/__init__.py", line 5, in <module>
celery-beat      |     from .dash import Dash, no_update  # noqa: F401,E402
celery-beat      |   File "/usr/local/lib/python3.8/site-packages/dash/dash.py", line 22, in <module>
celery-beat      |     from werkzeug.debug.tbtools import get_current_traceback
celery-beat      | ImportError: cannot import name 'get_current_traceback' from 'werkzeug.debug.tbtools' (/usr/local/lib/python3.8/site-packages/werkzeug/debug/tbtools.py)

Ref: plotly/dash#1993
@alexcjohnson
Copy link
Collaborator

Thanks @deepyaman - we ended up going with #1995 so I'll close this one but your work here was helpful toward fully understanding the issue!

@deepyaman deepyaman deleted the patch-1 branch March 29, 2022 21:29
@aitmlouk aitmlouk mentioned this pull request Mar 30, 2022
16 tasks
aitmlouk added a commit to aitmlouk/stackn that referenced this pull request Apr 2, 2022
Fix for ImportError: cannot import name 'get_current_traceback' from 'werkzeug.debug.tbtools' conflict with dash for studio

celery-beat      |     from .dash_demo import app
celery-beat      |   File "/app/monitor/dash_demo.py", line 8, in <module>
celery-beat      |     import dash
celery-beat      |   File "/usr/local/lib/python3.8/site-packages/dash/__init__.py", line 5, in <module>
celery-beat      |     from .dash import Dash, no_update  # noqa: F401,E402
celery-beat      |   File "/usr/local/lib/python3.8/site-packages/dash/dash.py", line 22, in <module>
celery-beat      |     from werkzeug.debug.tbtools import get_current_traceback
celery-beat      | ImportError: cannot import name 'get_current_traceback' from 'werkzeug.debug.tbtools' (/usr/local/lib/python3.8/site-packages/werkzeug/debug/tbtools.py)

Reproduce the error:
1- Install stackn (docker-compose up)

Ref: plotly/dash#1993
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ImportError: cannot import name 'get_current_traceback' from 'werkzeug.debug.tbtools'
4 participants