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.

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

Update twisted from 24.10.0 to 24.11.0 #8263

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Update twisted from 24.10.0 to 24.11.0 #8263

wants to merge 7 commits into from

Conversation

pmisik
Copy link
Contributor

@pmisik pmisik commented Dec 10, 2024

Contributor Checklist:

  • I have updated the unit tests
  • I have created a file in the newsfragments directory (and read the README.txt in that directory)
  • I have updated the appropriate documentation

@pmisik pmisik mentioned this pull request Dec 10, 2024
Merged
3 tasks
@pmisik
Copy link
Contributor Author

pmisik commented Dec 10, 2024

Do not merge yet.
It looks like mypy check is incorrectly implemented.
See errors in https://buildbot.buildbot.net/#/builders/148/builds/575/steps/9/logs/stdio

p12tic
p12tic reviewed Dec 10, 2024
View reviewed changes
worker/buildbot_worker/test/fake/reactor.py Outdated
@@ -83,7 +83,7 @@ def run(self) -> None:
def running(self) -> bool:
raise NotImplementedError()

def resolve(self, name: str, timeout: Sequence[int]) -> defer.Deferred[str]:
def resolve(self, name: str, timeout: Sequence[int] = (1, 3, 11, 45)) -> defer.Deferred[str]:
Copy link
Member

Choose a reason for hiding this comment

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

We should ignore this line in terms of typing because mypy will fail if other version of Twisted is installed (e.g. on developer machine).

@p12tic
Copy link
Member

p12tic commented Dec 10, 2024

It looks like mypy check is incorrectly implemented.

I think this PR should be able to go in still? It's not like it created these problems.

@pmisik
Copy link
Contributor Author

pmisik commented Dec 10, 2024 •
edited
Loading

I think this PR should be able to go in still? It's not like it created these problems.

This is correct.
I just put warning as I was surprised this PR got All checks have passed as I have seen errors in mypy errors on master while I "fixed" workers mypy errors.

It looks like https://github.com/buildbot/buildbot/blob/master/.bbtravis.yml#L186-L189

    cmd:  |
      export PATH=/tmp/bbvenv/bin/:$PATH
      (cd ./master && mypy --config-file ../pyproject.toml buildbot)
      (cd ./worker && mypy --config-file ../pyproject.toml buildbot_worker)

is evaluated in or manner while it should be evaluated in and manner

@p12tic
Copy link
Member

p12tic commented Dec 10, 2024

Right, it should have set -e beforehand.

@pmisik pmisik force-pushed the dep-update-2024-w48-twisted branch 2 times, most recently from d956f71 to 55ea7e7 Compare December 11, 2024 17:38
pyup-bot and others added 6 commits December 16, 2024 20:49
@pyup-bot @pmisik
Update twisted from 24.10.0 to 24.11.0
85e505d
@pyup-bot @pmisik
Update twisted from 24.10.0 to 24.11.0
7107d43
@pmisik
bbtravis: mypy: Add 'set -e' to ensure script exits on errors
8c98755
@pmisik
worker: ignore typing error. Twisted 24.11.0 changed type of IReactor…
3fd30c9 
…Core.resolve

Intent of this change is to keep compatibility with multiple twisted versions.

buildbot_worker/test/fake/reactor.py:86: error: Signature of "CoreReactor" incompatible with "resolve" of supertype
"IReactorCore"  [override]
        def resolve(self, name: str, timeout: Sequence[int]) -> defer.Deferred[str]:
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
buildbot_worker/test/fake/reactor.py:86: note:      Superclass:
buildbot_worker/test/fake/reactor.py:86: note:          def resolve(self, name: str, timeout: Sequence[int] = ...) -> Deferred[str]
buildbot_worker/test/fake/reactor.py:86: note:      Subclass:
buildbot_worker/test/fake/reactor.py:86: note:          def resolve(self, name: str, timeout: Sequence[int]) -> Deferred[str]
Found 1 error in 1 file (checked 73 source files)
@pmisik
Fix typing issue in master/buildbot/process/workerforbuilder.py
76ac859 
buildbot/process/workerforbuilder.py:181: error: Incompatible types in assignment (expression has type "States", variable has type "None") [assignment]
            self.state = States.DETACHED
                         ^~~~~~~~~~~~~~~
buildbot/process/workerforbuilder.py:203: error: Incompatible types in assignment (expression has type "States", variable has type "None") [assignment]
            self.state = States.AVAILABLE
                         ^~~~~~~~~~~~~~~~
@pmisik
buildbot/www/rest.py: Fix mypy error
5ae2507 
error: Incompatible types in assignment (expression
has type "bytes", variable has type "str")  [assignment]
            data = unicode2bytes(data)

Problem here seems to be how mypy tracks variable type.
Data is str before calling unicode2bytes.
But after it is bytes.
So probably simplest option is to use new variable name for bytes.
@pmisik pmisik force-pushed the dep-update-2024-w48-twisted branch from f176a94 to 5ae2507 Compare December 16, 2024 19:49
@pmisik
buildbot/worker/protocols/manager/pb.py: Fix empty tuple case handlin…
fc94ace 
…g in requestAvatar inline callback

buildbot/worker/protocols/manager/pb.py:58: error: Argument 1 to "bytes2unicode" has incompatible type "Union[bytes, Tuple[()]]"; expected "Union[bytes, str]"  [arg-type]
            avatarIdStr = bytes2unicode(avatarId)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@p12tic p12tic p12tic left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pmisik @p12tic @pyup-bot