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

Add code to rewrite Tchap Sygnal URL #69

Open
wants to merge 2 commits into
base: dinsic
Choose a base branch
from
Open

Conversation

MatMaul
Copy link
Contributor

@MatMaul MatMaul commented Nov 27, 2020

Copy link
Member

@giomfo giomfo left a comment

Choose a reason for hiding this comment

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

SGTM

@richvdh
Copy link
Member

richvdh commented Nov 30, 2020

could this not be done by updating the clients? Hardcoding the change into synapse seems like a poor choice.

@MatMaul
Copy link
Contributor Author

MatMaul commented Nov 30, 2020

could this not be done by updating the clients? Hardcoding the change into synapse seems like a poor choice.

So this will also done client side, but we don't control how/if people update their app.
We are also going to rewrite the push URL in existing pushers.

@richvdh
Copy link
Member

richvdh commented Dec 1, 2020

I'm definitely not enthusiastic about this sort of special-casing, but 🤷‍♂️

@MatMaul
Copy link
Contributor Author

MatMaul commented Dec 1, 2020

It's not ideal I agree, but we at least need to keep that for a while. I guess we can reassess that in ~6 months once we will be pretty sure that people have updated their client.

synapse/rest/client/v1/pusher.py Outdated Show resolved Hide resolved
@babolivier
Copy link
Contributor

babolivier commented Jul 13, 2021

@MatMaul What's the status on this? Is it still needed? If so I think it should use a configuration setting rather than being hardcoded, that way we can have it in mainline and prevent any divergence. Something like:

rewrite_http_pushers_url:
    - app_id: fr.gouv.btchap
      pusher_url: https://sygnal.preprod.tchap.gouv.fr

@MatMaul
Copy link
Contributor Author

MatMaul commented Jul 13, 2021

Good idea 👍 .

Pretty sure it is still needed unfortunately. I'll do that the proper way when this comes around.

@babolivier
Copy link
Contributor

Sounds good, my question was mostly to figure out in which state it was, by the sound of it it sounds like it's on standby from lack of demand but will likely come back on the table in the future.

@MatMaul MatMaul requested a review from a team as a code owner February 9, 2022 13:10
@babolivier babolivier removed the request for review from a team February 9, 2022 14:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants