Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

web_client_location documentation fix #2131

Merged
merged 3 commits into from Apr 27, 2017

Conversation

Projects
None yet
4 participants
Contributor

matthewjwolff commented Apr 17, 2017 edited

The config option web_client_location is very sparsely documented. In my case, I wanted to see if I could get the matrix homeserver to host the riot.im electron app sources. To remedy the lack in documentation, I

  1. Made the generated homeserver.yaml include information about this field. (synapse/config/server.py)

  2. Changed the Readme portion for Arch Linux users. The AngularJS sdk is not distributed with the synapse package, so running synapse with the default homeserver.yaml will fail on Arch if matrix-synpase is installed from the repo. This is documented on the Arch wiki but not here.

  3. Removed the string substitution from the error message in synapse/app/homeserver.py. The error message implies that it will only be printed if matrix-angular-sdk is not installed. But if matrix-angular-sdk is not installed, the program will crash when trying to perform the string substitution and not print the error message, defeating its purpose. If there's a better way to prevent this error, please enlighten me.

I saw that yall welcome additions to AUTHORS.rst. I feel like this fix isn't big enough for it to be appropriate to put my name next to the real developers. But hey if yall are fine with it I'd be happy to put my name there.

Signed-off-by: Matthew Wolff matthewjwolff@gmail.com

web_server_root documentation fix
Signed-off-by: Matthew Wolff <matthewjwolff@gmail.com>

Can one of the admins verify this patch?

Can one of the admins verify this patch?

Can one of the admins verify this patch?

Fixed travis build failure
Signed-off-by: Matthew Wolff <matthewjwolff@gmail.com>
synapse/app/homeserver.py
"\n"
"You can also disable hosting of the webclient via the\n"
"configuration option `web_client`\n"
- % {"dep": DEPENDENCY_LINKS["matrix-angular-sdk"]}
@APwhitehat

APwhitehat Apr 17, 2017

Contributor

String substitution could be failing because there is no key "matrix-angular-sdk" in DEPENDENCY_LINKS, see https://github.com/matrix-org/synapse/blob/master/synapse/python_dependencies.py#L76
Also it seems to be put into CONDITIONAL_REQUIREMENTS as "web_client".

I'm sort of new here so I could be wrong. 😛

@matthewjwolff

matthewjwolff Apr 17, 2017

Contributor

Yeah that is exactly why the string substitution fails. But I don't understand what the purpose of DEPENDENCY_LINKS is here. It seems to be an empty array whether matrix-angular-sdk is installed or not. Do things get added into that array in the course of server initialization? Maybe the thing that adds DEPENDENCY_LINKS["matrix-angular-sdk"] should occur earlier in server initialization?

@APwhitehat

APwhitehat Apr 18, 2017

Contributor

The purpose of DEPENDENCY_LINKS seems to be maintaining a list of all external dependencies which needs to be installed via url.
DEPENDENCY_LINKS is not initialized anywhere else afaik.
Also it seems to be static data like REQUIREMENTS & CONDITIONAL_REQUIREMENTS.

For your case you should probably link the "matrix-angular-sdk" from CONDITIONAL_REQUIREMENTS["web_client"]

Contributor

matthewjwolff commented Apr 18, 2017

I liked APwhitehat's suggestion

I'll recommend you not to mix two different kinds of changes in a single PR.It may cause confusion.
Perhaps indentation fixes in another PR?

Contributor

matthewjwolff commented Apr 18, 2017

I don't know how all those indentation changes got put in.... I definitely didn't mean to touch every file in the project...

Anyways I amended my previous commit to only the changes I originally intended to make. Sorry about that.

@matthewjwolff matthewjwolff changed the title from web_server_root documentation fix to web_client_location documentation fix Apr 18, 2017

Owner

erikjohnston commented Apr 25, 2017

Looks good! Did you manage to get it to host a riot instance in the end?

Contributor

matthewjwolff commented Apr 25, 2017

I did. And it was literally as easy as web_client_location: "/usr/lib/riot/webapp" (at least with how Arch packages the Riot electron app).

Contributor

matthewjwolff commented Apr 25, 2017 edited

I was going to investigate changing my riot instance so that the default homeserver is my domain instead of whatever Riot has as the default. I think the default is vector.im or matrix.org or something....

Owner

erikjohnston commented Apr 27, 2017

Cool! Thanks for this :)

@erikjohnston erikjohnston merged commit bb9246e into matrix-org:develop Apr 27, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

psaavedra added a commit to psaavedra/synapse that referenced this pull request May 19, 2017

Merge tag 'v0.21.0' into v0.21.0_no_federate_by_default
Changes in synapse v0.21.0 (2017-05-18)
=======================================

No changes since v0.21.0-rc3

Changes in synapse v0.21.0-rc3 (2017-05-17)
===========================================

Features:

* Add per user rate-limiting overrides (PR #2208)
* Add config option to limit maximum number of events requested by ``/sync``
  and ``/messages`` (PR #2221) Thanks to @psaavedra!

Changes:

* Various small performance fixes (PR #2201, #2202, #2224, #2226, #2227, #2228,
  #2229)
* Update username availability checker API (PR #2209, #2213)
* When purging, don't de-delta state groups we're about to delete (PR #2214)
* Documentation to check synapse version (PR #2215) Thanks to @hamber-dick!
* Add an index to event_search to speed up purge history API (PR #2218)

Bug fixes:

* Fix API to allow clients to upload one-time-keys with new sigs (PR #2206)

Changes in synapse v0.21.0-rc2 (2017-05-08)
===========================================

Changes:

* Always mark remotes as up if we receive a signed request from them (PR #2190)

Bug fixes:

* Fix bug where users got pushed for rooms they had muted (PR #2200)

Changes in synapse v0.21.0-rc1 (2017-05-08)
===========================================

Features:

* Add username availability checker API (PR #2183)
* Add read marker API (PR #2120)

Changes:

* Enable guest access for the 3pl/3pid APIs (PR #1986)
* Add setting to support TURN for guests (PR #2011)
* Various performance improvements (PR #2075, #2076, #2080, #2083, #2108,
  #2158, #2176, #2185)
* Make synctl a bit more user friendly (PR #2078, #2127) Thanks @APwhitehat!
* Replace HTTP replication with TCP replication (PR #2082, #2097, #2098,
  #2099, #2103, #2014, #2016, #2115, #2116, #2117)
* Support authenticated SMTP (PR #2102) Thanks @DanielDent!
* Add a counter metric for successfully-sent transactions (PR #2121)
* Propagate errors sensibly from proxied IS requests (PR #2147)
* Add more granular event send metrics (PR #2178)

Bug fixes:

* Fix nuke-room script to work with current schema (PR #1927) Thanks
  @zuckschwerdt!
* Fix db port script to not assume postgres tables are in the public schema
  (PR #2024) Thanks @jerrykan!
* Fix getting latest device IP for user with no devices (PR #2118)
* Fix rejection of invites to unreachable servers (PR #2145)
* Fix code for reporting old verify keys in synapse (PR #2156)
* Fix invite state to always include all events (PR #2163)
* Fix bug where synapse would always fetch state for any missing event (PR #2170)
* Fix a leak with timed out HTTP connections (PR #2180)
* Fix bug where we didn't time out HTTP requests to ASes  (PR #2192)

Docs:

* Clarify doc for SQLite to PostgreSQL port (PR #1961) Thanks @benhylau!
* Fix typo in synctl help (PR #2107) Thanks @HarHarLinks!
* ``web_client_location`` documentation fix (PR #2131) Thanks @matthewjwolff!
* Update README.rst with FreeBSD changes (PR #2132) Thanks @feld!
* Clarify setting up metrics (PR #2149) Thanks @encks!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment