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

Testing: Add type annotations to transfertool, mock and fts3 #6702

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

rdimaio
Copy link
Contributor

@rdimaio rdimaio commented Apr 19, 2024

Part of #6588.

This PR is a bit larger, the main bulk is fts3.py. The abstract methods defined in transfertool.py largely have the same type hints in their implementations in mock and fts3, with a few exceptions in cases where the sub-classes modified the types expected in the methods.

On the pyright failure:

 Found 1 new problems in /home/runner/work/rucio/rucio/lib/rucio/daemons/conveyor/receiver.py
  - 1 errors with message """Argument of type "Unknown | None" cannot be assigned to parameter "session" of type "Session" in function "get_db_fields_to_update"
                               Type "Unknown | None" is incompatible with type "Session"
                                 "None" is incompatible with "Session"""".
    Candidates: line 86

This is an issue within the Conveyor Receiver where we're passing a session that might be None to get_db_fields_to_update. I didn't want to potentially impact the functionality of this daemon, so I didn't add a check to ensure that session exists before passing it, but I think this check should probably exist in the daemon. I think it's best to keep this out of here to limit the scope of the PR to transfertool, but we might want to add it in the future.

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.

None yet

1 participant