Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Correctly pull twisted trunk when triggered overnight and ignore known type error #16115

Merged
merged 6 commits into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/twisted_trunk.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ on:
- cron: 0 8 * * *

workflow_dispatch:
# NB: inputs are only present when this workflow is dispatched manually.
# (The default below is the default field value in the form to trigger
# a manual dispatch). Otherwise the inputs will evaluate to null.
inputs:
twisted_ref:
description: Commit, branch or tag to checkout from upstream Twisted.
Expand Down Expand Up @@ -49,7 +52,7 @@ jobs:
extras: "all"
- run: |
poetry remove twisted
poetry add --extras tls git+https://github.com/twisted/twisted.git#${{ inputs.twisted_ref }}
poetry add --extras tls git+https://github.com/twisted/twisted.git#${{ inputs.twisted_ref || 'trunk' }}
poetry install --no-interaction --extras "all test"
- name: Remove warn_unused_ignores from mypy config
run: sed '/warn_unused_ignores = True/d' -i mypy.ini
Expand Down
1 change: 1 addition & 0 deletions changelog.d/16115.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Attempt to fix the twisted trunk job.
7 changes: 7 additions & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ warn_unused_ignores = False
disallow_untyped_defs = False
disallow_incomplete_defs = False

[mypy-synapse.util.manhole]
# This module imports something from Twisted which has a bad annotation in Twisted trunk,
# but is unannotated in Twisted's latest release. We want to type-ignore the problem
# in the twisted trunk job, even though it has no effect on normal mypy runs.
warn_unused_ignores = False


;; Dependencies without annotations
;; Before ignoring a module, check to see if type stubs are available.
;; The `typeshed` project maintains stubs here:
Expand Down
4 changes: 3 additions & 1 deletion synapse/util/manhole.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ def manhole(settings: ManholeConfig, globals: Dict[str, Any]) -> ServerFactory:
SynapseManhole, dict(globals, __name__="__console__")
)

factory = manhole_ssh.ConchFactory(portal.Portal(rlm, [checker]))
# type-ignore: This is an error in Twisted's annotations. See
# https://github.com/twisted/twisted/issues/11812 and /11813 .
factory = manhole_ssh.ConchFactory(portal.Portal(rlm, [checker])) # type: ignore[arg-type]
Comment on lines +101 to +103
Copy link
Contributor Author

@DMRobertson DMRobertson Aug 15, 2023

Choose a reason for hiding this comment

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

Urgh, the error is only introduced in twisted trunk, so adding this triggers an unused-ignore error on the current build.

Maybe I can use a cast instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I opted just to tell mypy to relax the unused-ignore check for this file. (Simpler, and I'm pretty sure it can complain about a redundant cast sometimes.)

Copy link
Member

Choose a reason for hiding this comment

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

Not a huge fan of adding an ignored file, can we do type: ignore[arg-type,<whatever the code is for an unneeded ignore>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I'm not sure it has an error code:

synapse/util/manhole.py:103: error: Unused "type: ignore" comment

Maybe I can just use a naked # type[ignore]?

Copy link
Member

Choose a reason for hiding this comment

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

Worth a try, if not :shipit:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No luck, I'm afraid. 🚢 ping it


# conch has the wrong type on these dicts (says bytes to bytes,
# should be bytes to Keys judging by how it's used).
Expand Down