Create basic admin command app#5597
Conversation
| @@ -201,6 +201,26 @@ def load_config(cls, description, argv): | |||
|
|
|||
There was a problem hiding this comment.
why don't we move load_config to a global function called load_config_for_worker_app?
There was a problem hiding this comment.
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.
synapse/config/_base.py
Outdated
| return obj | ||
|
|
||
| @classmethod | ||
| def create_argument_parser(cls, description): |
There was a problem hiding this comment.
probably makes more sense to construct the ArgumentParser in the caller and pass it in, making this add_arguments_to_parser?
There was a problem hiding this comment.
what would happen if we moved both create_argument_parser and load_config_with_parser to a derived class of HomeserverConfig?
There was a problem hiding this comment.
I'm not sure that would really buy us very much tbh?
| `config_parser.parse_args(..)` | ||
| """ | ||
|
|
||
| obj = cls() |
There was a problem hiding this comment.
might be better to do this in the caller?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I'm not sure, but happy to park this.
Codecov Report
@@ 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 Report
@@ 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 |
dc3fba1 to
c8f35d8
Compare
Co-Authored-By: Aaron Raimist <aaron@raim.ist>
richvdh
left a comment
There was a problem hiding this comment.
Generally seems sane. What does the --help look like>
| `config_parser.parse_args(..)` | ||
| """ | ||
|
|
||
| obj = cls() |
There was a problem hiding this comment.
I'm not sure, but happy to park this.
|
Help looks like: And |
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))
|
I am a little confused as of how to use this. Could you give me a usage example, please? |
I don't really remember why this was so complicated; I think it dates back to the time when we had to instantiate the Config classes before we could call `add_arguments` - ie before #5597. In any case, I don't think there's a good reason for it any more, and the impact of it being complicated is that `--help` doesn't work correctly.
I don't really remember why this was so complicated; I think it dates back to the time when we had to instantiate the Config classes before we could call `add_arguments` - ie before #5597. In any case, I don't think there's a good reason for it any more, and the impact of it being complicated is that `--help` doesn't work correctly.
I don't really remember why this was so complicated; I think it dates back to the time when we had to instantiate the Config classes before we could call `add_arguments` - ie before matrix-org#5597. In any case, I don't think there's a good reason for it any more, and the impact of it being complicated is that `--help` doesn't work correctly.
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-datathat 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.