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

Update default HS config to match well-known #11112

Merged
merged 5 commits into from Oct 10, 2019

Conversation

@erikjohnston
Copy link
Member

erikjohnston commented Oct 10, 2019

We want to move matrix.org client traffic to https://matrix-client.matrix.org

We want to move matrix.org client traffic to `https://matrix-client.matrix.org
@erikjohnston

This comment has been minimized.

Copy link
Member Author

erikjohnston commented Oct 10, 2019

Two things here:

  1. Its a little bit of a nuisance that riot-web can't just use the standard well-known for a server name.
  2. It would also be nice to have some mechanisms for telling clients they should change where they are talking to, somehow
@erikjohnston erikjohnston requested a review from vector-im/riot-web Oct 10, 2019
@turt2live

This comment has been minimized.

Copy link
Member

turt2live commented Oct 10, 2019

Its a little bit of a nuisance that riot-web can't just use the standard well-known for a server name.

We have default_server_name for this

@erikjohnston

This comment has been minimized.

Copy link
Member Author

erikjohnston commented Oct 10, 2019

Hmm, the docs say that that is deprecated? Or am I misreading it?

@turt2live

This comment has been minimized.

Copy link
Member

turt2live commented Oct 10, 2019

it does, but so is the thing riot.im has been serving to users for the last decade. It's deprecated because we would prefer people not use it, but ultimately we don't have a proper plan to remove it yet.

@jryans

This comment has been minimized.

Copy link
Member

jryans commented Oct 10, 2019

@turt2live Do you remember why we finally decided to prefer default_server_config style .well-known in Riot over WK from the HS? There's a use case solved by the Riot one, but I am forgetting what it is...

@erikjohnston

This comment has been minimized.

Copy link
Member Author

erikjohnston commented Oct 10, 2019

Any particular reason you'd prefer people not to use it? It seems odd to have a deprecated option that matrix.org ends up using

@jryans

This comment has been minimized.

Copy link
Member

jryans commented Oct 10, 2019

Indeed... We may want to stop labelling it deprecated if we are going to use it here. Maybe we need to summon @lampholder, as I can't recall if he had one style he'd prefer m.org to use.

@turt2live

This comment has been minimized.

Copy link
Member

turt2live commented Oct 10, 2019

we labelled it deprecated because we eventually don't want to support a bunch of different ways to define a server config. The default_server_config option was chosen as "best" because it has all the features of the other two that people are likely to care about, but misses the auto-updating part of it. It's also the most reliable if there were to be a server problem.

iirc, modular is (or should be) using the new default_server_config option over the other two.

@jryans

This comment has been minimized.

Copy link
Member

jryans commented Oct 10, 2019

We agreed as a group that default_server_name is the best fit for matrix.org's needs.

We should file a separate feature request to track the desire to switch over active sessions with the HS changes.

Copy link
Member

jryans left a comment

Looks good, but please also change:

"default_hs_url": "https://matrix.org",
"default_is_url": "https://vector.im",

@erikjohnston erikjohnston requested a review from jryans Oct 10, 2019
@jryans
jryans approved these changes Oct 10, 2019
Copy link
Member

jryans left a comment

Thanks! I'll deploy this to develop now.

@jryans jryans merged commit a0599e8 into develop Oct 10, 2019
3 of 5 checks passed
3 of 5 checks passed
buildkite/riot-web/pr Build #1095 started
Details
buildkite/riot-web/pr/karma-tests Started...
Details
buildkite/riot-web/pr/eslint-lint Passed (44 seconds)
Details
buildkite/riot-web/pr/i18n Passed (2 minutes, 32 seconds)
Details
buildkite/riot-web/pr/pipeline Passed (7 seconds)
Details
@jryans

This comment has been minimized.

Copy link
Member

jryans commented Oct 11, 2019

Deployed to app / staging as well, looking good. Electron will be updated with the next Riot release (planned for Monday).

turt2live added a commit to matrix-org/matrix-react-sdk that referenced this pull request Oct 23, 2019
This was missed in vector-im/riot-web#11112 and causes problems where matrix.org isn't pre-selected.
turt2live added a commit that referenced this pull request Oct 23, 2019
This was missed in #11112
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.