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.

Sign up for GitHub

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

Jump to bottom

fix type of 'session' function arguments #5947

Closed

rcarpa opened this issue Oct 27, 2022 · 0 comments

Assignees

Labels

Core & Internals enhancement

Milestone

Contributor

rcarpa commented Oct 27, 2022

Motivation

All over the code we use *_session decorators to automatically instantiate a session and pass it to the decorated function. To allow this automatic instantiation of the session, these functions have a session=None argument. This results in all static analyzers to be confused. They see an argument which clearly can take the value of 'None' and are not clever enough to understand that this value will be automatically instantiated by the decorator.

Modification

Make the session argument in the inner, decorated, functions mandatory.

The text was updated successfully, but these errors were encountered:

rcarpa added the Core & Internals label

rcarpa self-assigned this

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: make session a mandatory keyword-only argument. ruc…

440be61

…io#5947

The pyright type checker outputs 500-1000 errors of the format
"execute is not a member of None".

This behavior is correct. we explicitly mark the function as
accepting session=None, but then use it inside the function as if
it cannot be None. In reality, marking session as "Optional[Session]"
is actually completely wrong. The decorated function always expects
a valid session. However, the function returned by the decorator
has a different signature, it allows an empty session.

This commit makes Session mandatory for all functions decorated by
session decorators. To keep the risk of breaking code at minimum,
I avoid re-ordering function parameters. Hopefully, session is usually
the last argument and is almost always called as a keyword argument
in rucio. Enforce this behavior by making session (and logger)
a keyword-only argument. This way, it's possible to require a value
for this argument while keeping parameter reordering at minimum.

This change required to decorate did_meta functions with the session
decorators. Otherwise, calling those functions without session passed
raises an exception. It feels like those decorators where just
forgotten in the first place anyway.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: rework *_session decorators. Closes rucio#5947

6a1e063

Require 'session' to always be passed as a keyword argument to the
decorator and enforce this requirement by raising an exception.
Correctly update the signature of the returned function
("Session" -> "Optional[Session]"=None) and set the default value
of the 'session' kwarg to None.

There is currently no way to fully and correctly inform pyright about
this signature rewrite. That's why the type of decorators is not
entirely defined. I'm not sure how well the type deduction in pyright
will do its job, but I consider that it's more important to get rid
of the 500 + false positive errors in pyright related to session
management.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: make session a mandatory keyword-only argument. ruc…

…io#5947

The pyright type checker outputs 500-1000 errors of the format
"execute is not a member of None".

This behavior is correct. we explicitly mark the function as
accepting session=None, but then use it inside the function as if
it cannot be None. In reality, marking session as "Optional[Session]"
is actually completely wrong. The decorated function always expects
a valid session. However, the function returned by the decorator
has a different signature, it allows an empty session.

This commit makes Session mandatory for all functions decorated by
session decorators. To keep the risk of breaking code at minimum,
I avoid re-ordering function parameters. Hopefully, session is usually
the last argument and is almost always called as a keyword argument
in rucio. Enforce this behavior by making session (and logger)
a keyword-only argument. This way, it's possible to require a value
for this argument while keeping parameter reordering at minimum.

This change required to decorate did_meta functions with the session
decorators. Otherwise, calling those functions without session passed
raises an exception. It feels like those decorators where just
forgotten in the first place anyway.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: rework *_session decorators. Closes rucio#5947

a513434

Require 'session' to always be passed as a keyword argument to the
decorator and enforce this requirement by raising an exception.
Correctly update the signature of the returned function
("Session" -> "Optional[Session]"=None) and set the default value
of the 'session' kwarg to None.

There is currently no way to fully and correctly inform pyright about
this signature rewrite. That's why the type of decorators is not
entirely defined. I'm not sure how well the type deduction in pyright
will do its job, but I consider that it's more important to get rid
of the 500 + false positive errors in pyright related to session
management.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: make session a mandatory keyword-only argument. ruc…

381bb80

…io#5947

The pyright type checker outputs 500-1000 errors of the format
"execute is not a member of None".

This behavior is correct. we explicitly mark the function as
accepting session=None, but then use it inside the function as if
it cannot be None. In reality, marking session as "Optional[Session]"
is actually completely wrong. The decorated function always expects
a valid session. However, the function returned by the decorator
has a different signature, it allows an empty session.

This commit makes Session mandatory for all functions decorated by
session decorators. To keep the risk of breaking code at minimum,
I avoid re-ordering function parameters. Hopefully, session is usually
the last argument and is almost always called as a keyword argument
in rucio. Enforce this behavior by making session (and logger)
a keyword-only argument. This way, it's possible to require a value
for this argument while keeping parameter reordering at minimum.

This change required to decorate did_meta functions with the session
decorators. Otherwise, calling those functions without session passed
raises an exception. It feels like those decorators where just
forgotten in the first place anyway.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: rework *_session decorators. Closes rucio#5947

9ffaead

Require 'session' to always be passed as a keyword argument to the
decorator and enforce this requirement by raising an exception.
Correctly update the signature of the returned function
("Session" -> "Optional[Session]"=None) and set the default value
of the 'session' kwarg to None.

There is currently no way to fully and correctly inform pyright about
this signature rewrite. That's why the type of decorators is not
entirely defined. I'm not sure how well the type deduction in pyright
will do its job, but I consider that it's more important to get rid
of the 500 + false positive errors in pyright related to session
management.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: make session a mandatory keyword-only argument. ruc…

631f390

…io#5947

The pyright type checker outputs 500-1000 errors of the format
"execute is not a member of None".

This behavior is correct. we explicitly mark the function as
accepting session=None, but then use it inside the function as if
it cannot be None. In reality, marking session as "Optional[Session]"
is actually completely wrong. The decorated function always expects
a valid session. However, the function returned by the decorator
has a different signature, it allows an empty session.

This commit makes Session mandatory for all functions decorated by
session decorators. To keep the risk of breaking code at minimum,
I avoid re-ordering function parameters. Hopefully, session is usually
the last argument and is almost always called as a keyword argument
in rucio. Enforce this behavior by making session (and logger)
a keyword-only argument. This way, it's possible to require a value
for this argument while keeping parameter reordering at minimum.

This change required to decorate did_meta functions with the session
decorators. Otherwise, calling those functions without session passed
raises an exception. It feels like those decorators where just
forgotten in the first place anyway.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: rework *_session decorators. Closes rucio#5947

a9e8de0

Require 'session' to always be passed as a keyword argument to the
decorator and enforce this requirement by raising an exception.
Correctly update the signature of the returned function
("Session" -> "Optional[Session]"=None) and set the default value
of the 'session' kwarg to None.

There is currently no way to fully and correctly inform pyright about
this signature rewrite. That's why the type of decorators is not
entirely defined. I'm not sure how well the type deduction in pyright
will do its job, but I consider that it's more important to get rid
of the 500 + false positive errors in pyright related to session
management.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: make session a mandatory keyword-only argument. ruc…

8b68b3c

…io#5947

The pyright type checker outputs 500-1000 errors of the format
"execute is not a member of None".

This behavior is correct. we explicitly mark the function as
accepting session=None, but then use it inside the function as if
it cannot be None. In reality, marking session as "Optional[Session]"
is actually completely wrong. The decorated function always expects
a valid session. However, the function returned by the decorator
has a different signature, it allows an empty session.

This commit makes Session mandatory for all functions decorated by
session decorators. To keep the risk of breaking code at minimum,
I avoid re-ordering function parameters. Hopefully, session is usually
the last argument and is almost always called as a keyword argument
in rucio. Enforce this behavior by making session (and logger)
a keyword-only argument. This way, it's possible to require a value
for this argument while keeping parameter reordering at minimum.

This change required to decorate did_meta functions with the session
decorators. Otherwise, calling those functions without session passed
raises an exception. It feels like those decorators where just
forgotten in the first place anyway.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: rework *_session decorators. Closes rucio#5947

65cc537

Require 'session' to always be passed as a keyword argument to the
decorator and enforce this requirement by raising an exception.
Correctly update the signature of the returned function
("Session" -> "Optional[Session]"=None) and set the default value
of the 'session' kwarg to None.

There is currently no way to fully and correctly inform pyright about
this signature rewrite. That's why the type of decorators is not
entirely defined. I'm not sure how well the type deduction in pyright
will do its job, but I consider that it's more important to get rid
of the 500 + false positive errors in pyright related to session
management.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: make session a mandatory keyword-only argument. ruc…

7ae8596

…io#5947

The pyright type checker outputs 500-1000 errors of the format
"execute is not a member of None".

This behavior is correct. we explicitly mark the function as
accepting session=None, but then use it inside the function as if
it cannot be None. In reality, marking session as "Optional[Session]"
is actually completely wrong. The decorated function always expects
a valid session. However, the function returned by the decorator
has a different signature, it allows an empty session.

This commit makes Session mandatory for all functions decorated by
session decorators. To keep the risk of breaking code at minimum,
I avoid re-ordering function parameters. Hopefully, session is usually
the last argument and is almost always called as a keyword argument
in rucio. Enforce this behavior by making session (and logger)
a keyword-only argument. This way, it's possible to require a value
for this argument while keeping parameter reordering at minimum.

This change required to decorate did_meta functions with the session
decorators. Otherwise, calling those functions without session passed
raises an exception. It feels like those decorators where just
forgotten in the first place anyway.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: rework *_session decorators. Closes rucio#5947

2d1b3ad

Require 'session' to always be passed as a keyword argument to the
decorator and enforce this requirement by raising an exception.
Correctly update the signature of the returned function
("Session" -> "Optional[Session]"=None) and set the default value
of the 'session' kwarg to None.

There is currently no way to fully and correctly inform pyright about
this signature rewrite. That's why the type of decorators is not
entirely defined. I'm not sure how well the type deduction in pyright
will do its job, but I consider that it's more important to get rid
of the 500 + false positive errors in pyright related to session
management.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: make session a mandatory keyword-only argument. ruc…

a619b94

…io#5947

The pyright type checker outputs 500-1000 errors of the format
"execute is not a member of None".

This behavior is correct. we explicitly mark the function as
accepting session=None, but then use it inside the function as if
it cannot be None. In reality, marking session as "Optional[Session]"
is actually completely wrong. The decorated function always expects
a valid session. However, the function returned by the decorator
has a different signature, it allows an empty session.

This commit makes Session mandatory for all functions decorated by
session decorators. To keep the risk of breaking code at minimum,
I avoid re-ordering function parameters. Hopefully, session is usually
the last argument and is almost always called as a keyword argument
in rucio. Enforce this behavior by making session (and logger)
a keyword-only argument. This way, it's possible to require a value
for this argument while keeping parameter reordering at minimum.

This change required to decorate did_meta functions with the session
decorators. Otherwise, calling those functions without session passed
raises an exception. It feels like those decorators where just
forgotten in the first place anyway.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: rework *_session decorators. Closes rucio#5947

7fe721c

Require 'session' to always be passed as a keyword argument to the
decorator and enforce this requirement by raising an exception.
Correctly update the signature of the returned function
("Session" -> "Optional[Session]"=None) and set the default value
of the 'session' kwarg to None.

There is currently no way to fully and correctly inform pyright about
this signature rewrite. That's why the type of decorators is not
entirely defined. I'm not sure how well the type deduction in pyright
will do its job, but I consider that it's more important to get rid
of the 500 + false positive errors in pyright related to session
management.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: make session a mandatory keyword-only argument. ruc…

a846662

…io#5947

The pyright type checker outputs 500-1000 errors of the format
"execute is not a member of None".

This behavior is correct. we explicitly mark the function as
accepting session=None, but then use it inside the function as if
it cannot be None. In reality, marking session as "Optional[Session]"
is actually completely wrong. The decorated function always expects
a valid session. However, the function returned by the decorator
has a different signature, it allows an empty session.

This commit makes Session mandatory for all functions decorated by
session decorators. To keep the risk of breaking code at minimum,
I avoid re-ordering function parameters. Hopefully, session is usually
the last argument and is almost always called as a keyword argument
in rucio. Enforce this behavior by making session (and logger)
a keyword-only argument. This way, it's possible to require a value
for this argument while keeping parameter reordering at minimum.

This change required to decorate did_meta functions with the session
decorators. Otherwise, calling those functions without session passed
raises an exception. It feels like those decorators where just
forgotten in the first place anyway.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: rework *_session decorators. Closes rucio#5947

d0726be

Require 'session' to always be passed as a keyword argument to the
decorator and enforce this requirement by raising an exception.
Correctly update the signature of the returned function
("Session" -> "Optional[Session]"=None) and set the default value
of the 'session' kwarg to None.

There is currently no way to fully and correctly inform pyright about
this signature rewrite. That's why the type of decorators is not
entirely defined. I'm not sure how well the type deduction in pyright
will do its job, but I consider that it's more important to get rid
of the 500 + false positive errors in pyright related to session
management.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: make session a mandatory keyword-only argument. ruc…

91429db

…io#5947

The pyright type checker outputs 500-1000 errors of the format
"execute is not a member of None".

This behavior is correct. we explicitly mark the function as
accepting session=None, but then use it inside the function as if
it cannot be None. In reality, marking session as "Optional[Session]"
is actually completely wrong. The decorated function always expects
a valid session. However, the function returned by the decorator
has a different signature, it allows an empty session.

This commit makes Session mandatory for all functions decorated by
session decorators. To keep the risk of breaking code at minimum,
I avoid re-ordering function parameters. Hopefully, session is usually
the last argument and is almost always called as a keyword argument
in rucio. Enforce this behavior by making session (and logger)
a keyword-only argument. This way, it's possible to require a value
for this argument while keeping parameter reordering at minimum.

This change required to decorate did_meta functions with the session
decorators. Otherwise, calling those functions without session passed
raises an exception. It feels like those decorators where just
forgotten in the first place anyway.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: rework *_session decorators. Closes rucio#5947

5c80a7e

Require 'session' to always be passed as a keyword argument to the
decorator and enforce this requirement by raising an exception.
Correctly update the signature of the returned function
("Session" -> "Optional[Session]"=None) and set the default value
of the 'session' kwarg to None.

There is currently no way to fully and correctly inform pyright about
this signature rewrite. That's why the type of decorators is not
entirely defined. I'm not sure how well the type deduction in pyright
will do its job, but I consider that it's more important to get rid
of the 500 + false positive errors in pyright related to session
management.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: make session a mandatory keyword-only argument. ruc…

7f6c087

…io#5947

The pyright type checker outputs 500-1000 errors of the format
"execute is not a member of None".

This behavior is correct. we explicitly mark the function as
accepting session=None, but then use it inside the function as if
it cannot be None. In reality, marking session as "Optional[Session]"
is actually completely wrong. The decorated function always expects
a valid session. However, the function returned by the decorator
has a different signature, it allows an empty session.

This commit makes Session mandatory for all functions decorated by
session decorators. To keep the risk of breaking code at minimum,
I avoid re-ordering function parameters. Hopefully, session is usually
the last argument and is almost always called as a keyword argument
in rucio. Enforce this behavior by making session (and logger)
a keyword-only argument. This way, it's possible to require a value
for this argument while keeping parameter reordering at minimum.

This change required to decorate did_meta functions with the session
decorators. Otherwise, calling those functions without session passed
raises an exception. It feels like those decorators where just
forgotten in the first place anyway.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: rework *_session decorators. Closes rucio#5947

4a53896

Require 'session' to always be passed as a keyword argument to the
decorator and enforce this requirement by raising an exception.
Correctly update the signature of the returned function
("Session" -> "Optional[Session]"=None) and set the default value
of the 'session' kwarg to None.

There is currently no way to fully and correctly inform pyright about
this signature rewrite. That's why the type of decorators is not
entirely defined. I'm not sure how well the type deduction in pyright
will do its job, but I consider that it's more important to get rid
of the 500 + false positive errors in pyright related to session
management.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: make session a mandatory keyword-only argument. ruc…

9b8b4e3

…io#5947

The pyright type checker outputs 500-1000 errors of the format
"execute is not a member of None".

This behavior is correct. we explicitly mark the function as
accepting session=None, but then use it inside the function as if
it cannot be None. In reality, marking session as "Optional[Session]"
is actually completely wrong. The decorated function always expects
a valid session. However, the function returned by the decorator
has a different signature, it allows an empty session.

This commit makes Session mandatory for all functions decorated by
session decorators. To keep the risk of breaking code at minimum,
I avoid re-ordering function parameters. Hopefully, session is usually
the last argument and is almost always called as a keyword argument
in rucio. Enforce this behavior by making session (and logger)
a keyword-only argument. This way, it's possible to require a value
for this argument while keeping parameter reordering at minimum.

This change required to decorate did_meta functions with the session
decorators. Otherwise, calling those functions without session passed
raises an exception. It feels like those decorators where just
forgotten in the first place anyway.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: rework *_session decorators. Closes rucio#5947

635d61e

Require 'session' to always be passed as a keyword argument to the
decorator and enforce this requirement by raising an exception.
Correctly update the signature of the returned function
("Session" -> "Optional[Session]"=None) and set the default value
of the 'session' kwarg to None.

There is currently no way to fully and correctly inform pyright about
this signature rewrite. That's why the type of decorators is not
entirely defined. I'm not sure how well the type deduction in pyright
will do its job, but I consider that it's more important to get rid
of the 500 + false positive errors in pyright related to session
management.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: rework *_session decorators. Closes rucio#5947

9df6ac3

Require 'session' to always be passed as a keyword argument to the
decorator and enforce this requirement by raising an exception.
Correctly update the signature of the returned function
("Session" -> "Optional[Session]"=None) and set the default value
of the 'session' kwarg to None.

There is currently no way to fully and correctly inform pyright about
this signature rewrite. That's why the type of decorators is not
entirely defined. I'm not sure how well the type deduction in pyright
will do its job, but I consider that it's more important to get rid
of the 500 + false positive errors in pyright related to session
management.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: make session a mandatory keyword-only argument. ruc…

027a23b

…io#5947

The pyright type checker outputs 500-1000 errors of the format
"execute is not a member of None".

This behavior is correct. we explicitly mark the function as
accepting session=None, but then use it inside the function as if
it cannot be None. In reality, marking session as "Optional[Session]"
is actually completely wrong. The decorated function always expects
a valid session. However, the function returned by the decorator
has a different signature, it allows an empty session.

This commit makes Session mandatory for all functions decorated by
session decorators. To keep the risk of breaking code at minimum,
I avoid re-ordering function parameters. Hopefully, session is usually
the last argument and is almost always called as a keyword argument
in rucio. Enforce this behavior by making session (and logger)
a keyword-only argument. This way, it's possible to require a value
for this argument while keeping parameter reordering at minimum.

This change required to decorate did_meta functions with the session
decorators. Otherwise, calling those functions without session passed
raises an exception. It feels like those decorators where just
forgotten in the first place anyway.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: rework *_session decorators. Closes rucio#5947

38e489b

Require 'session' to always be passed as a keyword argument to the
decorator and enforce this requirement by raising an exception.
Correctly update the signature of the returned function
("Session" -> "Optional[Session]"=None) and set the default value
of the 'session' kwarg to None.

There is currently no way to fully and correctly inform pyright about
this signature rewrite. That's why the type of decorators is not
entirely defined. I'm not sure how well the type deduction in pyright
will do its job, but I consider that it's more important to get rid
of the 500 + false positive errors in pyright related to session
management.

rcarpa added a commit to rcarpa/rucio that referenced this issue


          Core & Internals: make session a mandatory keyword-only argument. ruc…

6fe86be

…io#5947

The pyright type checker outputs 500-1000 errors of the format
"execute is not a member of None".

This behavior is correct. we explicitly mark the function as
accepting session=None, but then use it inside the function as if
it cannot be None. In reality, marking session as "Optional[Session]"
is actually completely wrong. The decorated function always expects
a valid session. However, the function returned by the decorator
has a different signature, it allows an empty session.

This commit makes Session mandatory for all functions decorated by
session decorators. To keep the risk of breaking code at minimum,
I avoid re-ordering function parameters. Hopefully, session is usually
the last argument and is almost always called as a keyword argument
in rucio. Enforce this behavior by making session (and logger)
a keyword-only argument. This way, it's possible to require a value
for this argument while keeping parameter reordering at minimum.

This change required to decorate did_meta functions with the session
decorators. Otherwise, calling those functions without session passed
raises an exception. It feels like those decorators where just
forgotten in the first place anyway.

bari12 closed this as completed in

cfe3aaf

bari12 added a commit that referenced this issue


          Merge pull request #5948 from rcarpa/feature-5947-session_type

1a09787

Core & Internals: make session a mandatory keyword-only argument. Closes #5947

bari12 added the enhancement label

bari12 added this to the 1.30.0 "The Donkeynator" milestone

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