Skip to content

Fix Sytest job failing on daily Twisted trunk workflow - #20293

Merged
anoadragon453 merged 3 commits into
developfrom
anoa/fix_sytest_twisted_trunk
Oct 6, 2026
Merged

anoadragon453 merged 3 commits into
developfrom
anoa/fix_sytest_twisted_trunk

Conversation

@anoadragon453

@anoadragon453 anoadragon453 commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

Addresses the first portion of #20292. See that issue for the full context.

Broke in #19234.

The run now succeeds when run against this PR's branch: https://github.com/element-hq/synapse/actions/runs/36712766021

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct (run the linters)

Comment thread .github/workflows/twisted_trunk.yml
@anoadragon453
anoadragon453 marked this pull request as ready for review September 30, 2026 12:33
@anoadragon453
anoadragon453 requested a review from a team as a code owner September 30, 2026 12:33
@anoadragon453
anoadragon453 requested review from MadLittleMods and removed request for a team September 30, 2026 12:33
Comment thread .github/workflows/twisted_trunk.yml
Comment on lines -141 to -143
# Use offline mode to avoid reinstalling the pinned version of
# twisted.
OFFLINE: 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why did we ever use OFFLINE: 1 if it isn't necessary?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Looks like setting this option was actually done unnecessarily by matrix-org/synapse#12425. Doing so was useful in an earlier draft of matrix-org/synapse#12337 (which 12425 was extracted from). That earlier draft contained:

poetry install --no-interaction --extras "all test"
poetry run pip install git+https://github.com/twisted/twisted.git@trunk

in the .ci/patch_for_twisted_trunk.sh script.

This installs the normal locked dependencies via poetry, then replaced Twisted using pip. It did not update pyproject.toml or poetry.lock. The reason given in the script was:

# ... we run into https://github.com/python-poetry/poetry/issues/5311, where
# poetry insists on installing an old version of treq, which isn't actually compatible
# with recent twisted releases. So let's just install twisted trunk using pip.

pip was used to install Twisted trunk instead of poetry. With that, running poetry install again would override the Twisted version pip installed. Thus, OFFLINE: 1 was used to prevent that second poetry install.

Later, the pip invocation was removed, and replaced with poetry install again. But the OFFLINE: 1 was left over (and unnecessary).

The OFFLINE switch itself originated from matrix-org/sytest#588. So we may still want to keep it, even if no CI references it anymore; if we deem the use case worthwhile.

@anoadragon453

Copy link
Copy Markdown
Member Author

Re-ran the Twisted Trunk job on this branch and it still succeeds: https://github.com/element-hq/synapse/actions/runs/37448667754

Thanks for the review @MadLittleMods!

@anoadragon453
anoadragon453 merged commit d92b0b4 into develop Oct 6, 2026
44 checks passed
@anoadragon453
anoadragon453 deleted the anoa/fix_sytest_twisted_trunk branch October 6, 2026 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants