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

Create basic admin command app #5597

Merged
merged 13 commits into from Jul 16, 2019

Conversation

@erikjohnston
Copy link
Member

commented Jul 2, 2019

Add a basic app that is meant to be run as a "command". This lets server admins run functions as a command (using their existing config) without having to use an admin API. This has the advantage that resource intensive actions can be run separately from production synapse instances.

As a starting point this supports a single command export-data that exposes the functionality from #5589.

Note: This doesn't hook into replication at all, so caches will not be correctly invalidated. I think this is fine given the intention of this tool, though we might want to consider turning off caching entirely to ensure we have a consistent view?

Based on #5589.

erikjohnston added 2 commits Jul 1, 2019

@erikjohnston erikjohnston requested a review from matrix-org/synapse-core Jul 2, 2019

@richvdh
Copy link
Member

left a comment

some thoughts on all this...

synapse/config/_base.py Outdated Show resolved Hide resolved
synapse/app/admin_cmd.py Outdated Show resolved Hide resolved
synapse/config/_base.py Outdated Show resolved Hide resolved
@@ -201,6 +201,26 @@ def load_config(cls, description, argv):

This comment has been minimized.

Copy link
@richvdh

richvdh Jul 3, 2019

Member

why don't we move load_config to a global function called load_config_for_worker_app?

This comment has been minimized.

Copy link
@erikjohnston

erikjohnston Jul 15, 2019

Author Member

Could do, though I tend to think of the load_config* functions as creators of HomeserverConfig, so to me feels natural they'd be part of the class.

return obj

@classmethod
def create_argument_parser(cls, description):

This comment has been minimized.

Copy link
@richvdh

richvdh Jul 3, 2019

Member

probably makes more sense to construct the ArgumentParser in the caller and pass it in, making this add_arguments_to_parser?

This comment has been minimized.

Copy link
@richvdh

richvdh Jul 3, 2019

Member

what would happen if we moved both create_argument_parser and load_config_with_parser to a derived class of HomeserverConfig?

This comment has been minimized.

Copy link
@erikjohnston

erikjohnston Jul 15, 2019

Author Member

I'm not sure that would really buy us very much tbh?

`config_parser.parse_args(..)`
"""

obj = cls()

This comment has been minimized.

Copy link
@richvdh

richvdh Jul 3, 2019

Member

might be better to do this in the caller?

This comment has been minimized.

Copy link
@erikjohnston

erikjohnston Jul 15, 2019

Author Member

I'm not fond of the idea that callers need to create a blank object and then call a function to properly instantiate it, rather than just call a class method that gives back a fully instantiated object.

This comment has been minimized.

Copy link
@richvdh

richvdh Jul 16, 2019

Member

I'm not sure, but happy to park this.

synapse/app/admin_cmd.py Outdated Show resolved Hide resolved
synapse/app/admin_cmd.py Show resolved Hide resolved
synapse/app/admin_cmd.py Show resolved Hide resolved

@erikjohnston erikjohnston added this to In progress in Homeserver Task Board via automation Jul 4, 2019

@codecov

This comment has been minimized.

Copy link

commented Jul 15, 2019

Codecov Report

Merging #5597 into develop will increase coverage by 0.04%.
The diff coverage is 63.39%.

@@            Coverage Diff             @@
##           develop   #5597      +/-   ##
==========================================
+ Coverage    63.16%   63.2%   +0.04%     
==========================================
  Files          328     328              
  Lines        35930   36011      +81     
  Branches      5917    5934      +17     
==========================================
+ Hits         22694   22762      +68     
- Misses       11609   11624      +15     
+ Partials      1627    1625       -2
@codecov

This comment has been minimized.

Copy link

commented Jul 15, 2019

Codecov Report

Merging #5597 into develop will increase coverage by 0.01%.
The diff coverage is 91.3%.

@@             Coverage Diff             @@
##           develop    #5597      +/-   ##
===========================================
+ Coverage    63.34%   63.35%   +0.01%     
===========================================
  Files          331      331              
  Lines        36129    36140      +11     
  Branches      5952     5954       +2     
===========================================
+ Hits         22885    22898      +13     
+ Misses       11613    11611       -2     
  Partials      1631     1631

@erikjohnston erikjohnston force-pushed the erikj/admin_api_cmd branch from dc3fba1 to c8f35d8 Jul 15, 2019

@erikjohnston erikjohnston requested a review from richvdh Jul 15, 2019

synapse/app/admin_cmd.py Outdated Show resolved Hide resolved
Fix typo in synapse/app/admin_cmd.py
Co-Authored-By: Aaron Raimist <aaron@raim.ist>
@richvdh
Copy link
Member

left a comment

Generally seems sane. What does the --help look like>

synapse/config/_base.py Outdated Show resolved Hide resolved
synapse/config/_base.py Outdated Show resolved Hide resolved
`config_parser.parse_args(..)`
"""

obj = cls()

This comment has been minimized.

Copy link
@richvdh

richvdh Jul 16, 2019

Member

I'm not sure, but happy to park this.

@erikjohnston

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2019

Help looks like:

$ python -m synapse.app.admin_cmd --help
usage: admin_cmd.py [-h] [-c CONFIG_FILE] [--keys-directory DIRECTORY] [-D]
                    [--print-pidfile] [--manhole PORT]
                    [-d SQLITE_DATABASE_PATH] [-v] [-f LOG_FILE]
                    [--log-config LOG_CONFIG] [-n] [--enable-registration]
                    <admin_command> ...
Synapse Admin Command
optional arguments:
  -h, --help            show this help message and exit
  -c CONFIG_FILE, --config-path CONFIG_FILE
                        Specify config file. Can be given multiple times and
                        may specify directories containing *.yaml files.
  --keys-directory DIRECTORY
                        Where files such as certs and signing keys are stored
                        when their location is not given explicitly in the
                        config. Defaults to the directory containing the last
                        config file
server:
  -D, --daemonize       Daemonize the home server
  --print-pidfile       Print the path to the pidfile just before daemonizing
  --manhole PORT        Turn on the twisted telnet manhole service on the
                        given port.
database:
  -d SQLITE_DATABASE_PATH, --database-path SQLITE_DATABASE_PATH
                        The path to a sqlite database to use.
logging:
  -v, --verbose         The verbosity level. Specify multiple times to
                        increase verbosity. (Ignored if --log-config is
                        specified.)
  -f LOG_FILE, --log-file LOG_FILE
                        File to log to. (Ignored if --log-config is
                        specified.)
  --log-config LOG_CONFIG
                        Python logging config file
  -n, --no-redirect-stdio
                        Do not redirect stdout/stderr to the log
registration:
  --enable-registration
                        Enable registration for new users.
Admin Commands:
  Choose an admin command to perform.
  <admin_command>       The admin command to perform.
    export-data         Export all data for a user

And

$ python -m synapse.app.admin_cmd export-data  --help
usage: admin_cmd.py export-data [-h] [--output-directory DIRECTORY] user_id
positional arguments:
  user_id               User to extra data from
optional arguments:
  -h, --help            show this help message and exit
  --output-directory DIRECTORY
                        The directory to store the exported data in. Must be
                        emtpy. Defaults to creating a temp directory.

@erikjohnston erikjohnston requested a review from richvdh Jul 16, 2019

@richvdh
Copy link
Member

left a comment

lgtm

@erikjohnston erikjohnston merged commit c831c5b into develop Jul 16, 2019

19 checks passed

buildkite/synapse Build #2828 passed (19 minutes, 30 seconds)
Details
buildkite/synapse/check-sample-config Passed (1 minute, 16 seconds)
Details
buildkite/synapse/check-style Passed (1 minute, 14 seconds)
Details
buildkite/synapse/isort Passed (23 seconds)
Details
buildkite/synapse/newspaper-newsfile Passed (18 seconds)
Details
buildkite/synapse/packaging Passed (23 seconds)
Details
buildkite/synapse/pipeline Passed (10 seconds)
Details
buildkite/synapse/python-3-dot-5-slash-postgres-9-dot-5 Passed (17 minutes, 59 seconds)
Details
buildkite/synapse/python-3-dot-5-slash-sqlite Passed (4 minutes, 20 seconds)
Details
buildkite/synapse/python-3-dot-5-slash-sqlite-slash-old-deps Passed (5 minutes, 54 seconds)
Details
buildkite/synapse/python-3-dot-6-slash-sqlite Passed (4 minutes, 36 seconds)
Details
buildkite/synapse/python-3-dot-7-slash-postgres-11 Passed (17 minutes, 41 seconds)
Details
buildkite/synapse/python-3-dot-7-slash-postgres-9-dot-5 Passed (17 minutes, 35 seconds)
Details
buildkite/synapse/python-3-dot-7-slash-sqlite Passed (4 minutes, 30 seconds)
Details
buildkite/synapse/sytest-python-3-dot-5-slash-postgres-9-dot-6-slash-monolith Passed (6 minutes, 2 seconds)
Details
buildkite/synapse/sytest-python-3-dot-5-slash-postgres-9-dot-6-slash-workers Soft failed (exit status 1)
Details
buildkite/synapse/sytest-python-3-dot-5-slash-sqlite-slash-monolith Passed (4 minutes, 26 seconds)
Details
codecov/patch 91.3% of diff hit (target 0%)
Details
codecov/project 63.35% (target 0%)
Details

Homeserver Task Board automation moved this from In progress to Done Jul 16, 2019

anoadragon453 added a commit that referenced this pull request Jul 22, 2019
Merge tag 'v1.2.0rc1' into develop
v1.2.0rc1

Features
--------

- Add support for opentracing. ([\#5544](#5544), [\#5712](#5712))
- Add ability to pull all locally stored events out of synapse that a particular user can see. ([\#5589](#5589))
- Add a basic admin command app to allow server operators to run Synapse admin commands separately from the main production instance. ([\#5597](#5597))
- Add `sender` and `origin_server_ts` fields to `m.replace`. ([\#5613](#5613))
- Add default push rule to ignore reactions. ([\#5623](#5623))
- Include the original event when asking for its relations. ([\#5626](#5626))
- Implement `session_lifetime` configuration option, after which access tokens will expire. ([\#5660](#5660))
- Return "This account has been deactivated" when a deactivated user tries to login. ([\#5674](#5674))
- Enable aggregations support by default ([\#5714](#5714))

Bugfixes
--------

- Fix 'utime went backwards' errors on daemonization. ([\#5609](#5609))
- Various minor fixes to the federation request rate limiter. ([\#5621](#5621))
- Forbid viewing relations on an event once it has been redacted. ([\#5629](#5629))
- Fix requests to the `/store_invite` endpoint of identity servers being sent in the wrong format. ([\#5638](#5638))
- Fix newly-registered users not being able to lookup their own profile without joining a room. ([\#5644](#5644))
- Fix bug in #5626 that prevented the original_event field from actually having the contents of the original event in a call to `/relations`. ([\#5654](#5654))
- Fix 3PID bind requests being sent to identity servers as `application/x-form-www-urlencoded` data, which is deprecated. ([\#5658](#5658))
- Fix some problems with authenticating redactions in recent room versions. ([\#5699](#5699), [\#5700](#5700), [\#5707](#5707))
- Ignore redactions of m.room.create events. ([\#5701](#5701))

Updates to the Docker image
---------------------------

- Base Docker image on a newer Alpine Linux version (3.8 -> 3.10). ([\#5619](#5619))
- Add missing space in default logging file format generated by the Docker image. ([\#5620](#5620))

Improved Documentation
----------------------

- Add information about nginx normalisation to reverse_proxy.rst. Contributed by @skalarproduktraum - thanks! ([\#5397](#5397))
- --no-pep517 should be --no-use-pep517 in the documentation to setup the development environment. ([\#5651](#5651))
- Improvements to Postgres setup instructions. Contributed by @Lrizika - thanks! ([\#5661](#5661))
- Minor tweaks to postgres documentation. ([\#5675](#5675))

Deprecations and Removals
-------------------------

- Remove support for the `invite_3pid_guest` configuration setting. ([\#5625](#5625))

Internal Changes
----------------

- Move logging code out of `synapse.util` and into `synapse.logging`. ([\#5606](#5606), [\#5617](#5617))
- Add a blacklist file to the repo to blacklist certain sytests from failing CI. ([\#5611](#5611))
- Make runtime errors surrounding password reset emails much clearer. ([\#5616](#5616))
- Remove dead code for persiting outgoing federation transactions. ([\#5622](#5622))
- Add `lint.sh` to the scripts-dev folder which will run all linting steps required by CI. ([\#5627](#5627))
- Move RegistrationHandler.get_or_create_user to test code. ([\#5628](#5628))
- Add some more common python virtual-environment paths to the black exclusion list. ([\#5630](#5630))
- Some counter metrics exposed over Prometheus have been renamed, with the old names preserved for backwards compatibility and deprecated. See `docs/metrics-howto.rst` for details. ([\#5636](#5636))
- Unblacklist some user_directory sytests. ([\#5637](#5637))
- Factor out some redundant code in the login implementation. ([\#5639](#5639))
- Update ModuleApi to avoid register(generate_token=True). ([\#5640](#5640))
- Remove access-token support from `RegistrationHandler.register`, and rename it. ([\#5641](#5641))
- Remove access-token support from `RegistrationStore.register`, and rename it. ([\#5642](#5642))
- Improve logging for auto-join when a new user is created. ([\#5643](#5643))
- Remove unused and unnecessary check for FederationDeniedError in _exception_to_failure. ([\#5645](#5645))
- Fix a small typo in a code comment. ([\#5655](#5655))
- Clean up exception handling around client access tokens. ([\#5656](#5656))
- Add a mechanism for per-test homeserver configuration in the unit tests. ([\#5657](#5657))
- Inline issue_access_token. ([\#5659](#5659))
- Update the sytest BuildKite configuration to checkout Synapse in `/src`. ([\#5664](#5664))
- Add a `docker` type to the towncrier configuration. ([\#5673](#5673))
- Convert `synapse.federation.transport.server` to `async`. Might improve some stack traces. ([\#5689](#5689))
- Documentation for opentracing. ([\#5703](#5703))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.