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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assigning to attribute flask.g.[...] not defined in class slots (assigning-non-slot) #4020

Closed
anders-kiaer opened this issue May 12, 2021 · 16 comments 路 Fixed by #4042
Closed
Assignees
Labels
Milestone

Comments

@anders-kiaer
Copy link

Thanks for a recent release of flask which looks great. 馃帀 Just started testing it out.

This is not directly a bug afaik, but our CI started failing on the new release of flask and werkzeug.

Following the flask documentation, it appears the recommended way of setting new attributes to flask.g is simply doing flask.g.some_attribute = some_value.

This example code

from flask import g

g.some_attribute = 42

gives no pylint warning when used together with pip install "flask<2" "werkzeug<2", however after pip install --upgrade flask werkzeug the same code gives the following pylint warning:

test_pylint.py:3:0: E0237: Assigning to attribute 'some_attribute' not defined in class slots (assigning-non-slot)

Any recommended code change that should be done related to setting attributes on the namespace object flask.g, or would the best recommendation be to silence pylint when used with newest flask + werkzeug?

@davidism
Copy link
Member

davidism commented May 12, 2021

g is a proxy object, and it is unbound in your example. Assigning to it is not doing what you expect even in previous versions. You can only use g within an application or request context.

@app.before_request
def set_attr():
    g.some_attribute = 42

Or if you really need to test this without a request:

with app.app_context():
    g.some_attribute = 42

@anders-kiaer
Copy link
Author

@davidism Thanks for quick answer.

We are in practice doing it in an @app.before_request. I just moved it out in the snippet above in order to make a MWE when it comes to the pylint warning/feedback. In other words, using it inside an application/request context (also) raises the pylint warning.

@davidism
Copy link
Member

That sounds like an issue with pylint. Unfortunately, LocalProxy is pretty magical, so it's not unexpected that static analysis tools might have trouble with it.

@hughsie
Copy link

hughsie commented May 13, 2021

That sounds like an issue with pylint

Note, it also causes a failure in mypy with "AppContext" has no attribute "some_attribute"...

@davidism
Copy link
Member

It looks like the type of flask.globals.g is incorrectly marked as AppContext instead of _AppCtxGlobals, which I can fix. However, neither of them set __slots__, so I'm not sure why that's the error.

@davidism davidism reopened this May 13, 2021
@hughsie
Copy link

hughsie commented May 13, 2021

@davidism if you push a test branch I'd be happy to test with my app.

@davidism
Copy link
Member

davidism commented May 13, 2021

Ah, the issue will still be with LocalProxy, since it does set __slots__, and that's what the g object really is. That's probably still going to have to be reported to mypy and pylint. The fact that it sets __slots__ is not new, and it defines __setattr__, so not sure why the error started.

@davidism
Copy link
Member

davidism commented May 13, 2021

The __slots__ issue needs to be reported to pylint.

The "has no attribute" issue needs to be reported to mypy.

It seems like both should allow any attribute since the classes define __getattr__ and __setattr__. There's no good way for us to get them to ignore these errors from our code. For mypy, we could mark the object as Any, but then you wouldn't get the benefit of introspection/checking the methods that it has.

I updated the type annotation, but both messages are still produced, I don't think there's anything we can do.

@hughsie
Copy link

hughsie commented May 13, 2021

I don't think there's anything we can do

I think anyone using either mypy or pylint is going to hit this on basically any project using Flask; most tutorials just tell you to create random properties on g and I fear the number of dupes for this issue is going to be huge when people start upgrading. Unfortunately I don't have the python experience to work this out. :(

@lucaswerkmeister
Copy link
Contributor

It looks like the type of flask.globals.g is incorrectly marked as AppContext instead of _AppCtxGlobals, which I can fix.

I assume this is also the reason for a mypy error I鈥檓 seeing since upgrading to Flask 2.0.0:

app.py:390: error: "AppContext" has no attribute "setdefault"

_AppCtxGlobals has setdefault, AppContext doesn鈥檛.

@davidism
Copy link
Member

davidism commented May 13, 2021

After experimenting, it looks like mypy understands that an object is a namespace for arbitrary attributes if __getattr__, etc. are defined, so I can add that to the globals class and mypy should stop warning about that.

@pgjones
Copy link
Member

pgjones commented May 13, 2021

Yea, the SimpleNamespace is typed this way. I'm experimenting with Quart at the moment.

@davidism davidism self-assigned this May 13, 2021
@davidism davidism added this to the 2.0.1 milestone May 13, 2021
@davidism davidism mentioned this issue May 13, 2021
3 tasks
@hughsie
Copy link

hughsie commented May 13, 2021

@davidism this fixed the CI on the LVFS, thanks!

@surkova
Copy link

surkova commented May 24, 2021

I'm in a similar seats here, trying to upgrade to 2.0.1 though:

    @app.before_request
    def before_request():
        flask.g.start_time = time.time()

Getting

E0237: Assigning to attribute 'start_time' not defined in class slots (assigning-non-slot)

We are using attribute assignment to figure out how long the request takes and report it to datadog.

Any ideas on what we should be doing instead since it's 2.0.1 and not fixed for us yet?

@davidism
Copy link
Member

Yes, that's a pylint error, not a mypy error. Nothing we can do about it, report it to pylint.

@anders-kiaer
Copy link
Author

anders-kiaer commented May 27, 2021

Ah, the issue will still be with LocalProxy, since it does set __slots__, and that's what the g object really is. That's probably still going to have to be reported to mypy and pylint. The fact that it sets __slots__ is not new, and it defines __setattr__, so not sure why the error started.

FWIW and future reference (if to be reported in pylint), it looks like the reason the pylint warning started appearing is due to LocalProxy's __slots__ contains "__dict__" on werkzeug<2 and does not contain "__dict__" on werkzeug>=2.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants