This repository was archived by the owner on Apr 26, 2024. It is now read-only.
Repository navigation
This repository was archived by the owner on Apr 26, 2024. It is now read-only.
Export data admin command is broken. #10908
Description
Running python -m synapse.app.admin_cmd -c <config_file> export-data <user_id> results in:
Traceback (most recent call last):
File "/usr/local/lib/python3.8/runpy.py", line 194, in _run_module_as_main
return _run_code(code, main_globals, None,
File "/usr/local/lib/python3.8/runpy.py", line 87, in _run_code
exec(code, run_globals)
File "/usr/local/lib/python3.8/site-packages/synapse/app/admin_cmd.py", line 236, in <module>
start(sys.argv[1:])
File "/usr/local/lib/python3.8/site-packages/synapse/app/admin_cmd.py", line 190, in start
and not config.worker_log_file
File "/usr/local/lib/python3.8/site-packages/synapse/config/_base.py", line 340, in __getattr__
return self._get_unclassed_config(None, item)
File "/usr/local/lib/python3.8/site-packages/synapse/config/_base.py", line 363, in _get_unclassed_config
raise AttributeError(item, "not found in %s" % (list(self._configs.keys()),))
AttributeError: ('worker_log_file', "not found in ['modules', 'server', 'tls', 'federation', 'caches', 'database', 'logging', 'ratelimiting', 'media', 'oembed', 'captcha', 'voip', 'registration', 'account_validity', 'metrics', 'api', 'appservice', 'key', 'saml2', 'oidc', 'cas', 'sso', 'jwt', 'auth', 'email', 'authproviders', 'push', 'spamchecker', 'room', 'groups', 'userdirectory', 'consent', 'stats', 'servernotices', 'roomdirectory', 'thirdpartyrules', 'tracing', 'worker', 'redis', 'experimental']")
The effect of this is that its currently not possible to export a users data, e.g due to a GDPR request.
c.f. #10900
Activity
dklimpel commentedon Sep 28, 2021
When I am ty to fix it by removing this line:
synapse/synapse/app/admin_cmd.py
Line 190 in 8aaa4b7
I get this new error:
@erikjohnston Do you have a hint to fix it? Is it a missing import?
H-Shay commentedon Oct 1, 2021
It looks like the issue is that we recently changed
config.worker_log_filetoconfig.worker.worker_log_fileinsynapse/synapse/app/admin_cmd.pyhowever, theconfig.workerobject has no attributeworker_log_file, nor is there any references to aworker_log_fileanywhere else in the code. A little digging did uncover that the logging configuration file for a worker will contain a filename/path to a logfile for that worker.My question is, since we are already checking that the
config.worker.worker_log_configexists, do we really also need to check that the log file that it presumably references exists as well? I.e. can we drop that line, or is it necessary to figure out how to extract the reference to the worker log file from theworker_log_configand check for it when this is the only place that reference will be used? I spent some time trying to do just that and couldn't figure it out, which led me to wonder if it isn't necessary in the first place.DMRobertson commentedon Oct 5, 2021
Indeed, but I think this goes back further than the recent changes you mention (#10897?).
Running
the first two hits are bb7fdd8 and 6a85cb5. The former is part of Patrick's PR; the latter is part of (the pleasingly-numberered) #5678 from 2019.
Has this script been broken since 2019? Surely we'd have noticed sooner...
I think so. 6a85cb5 seems to be removing
log_fileconfigurations. And in e484101 we explicitly said "this option is gone".So yeah, my vote is to drop that line.
DMRobertson commentedon Oct 5, 2021
(Though as @dklimpel notes, there might be more fixing required afterwards.)
richvdh commentedon Oct 5, 2021
seems like it has, and nobody at element uses it. If we're going to have it, we should probably have some CI to check it keeps working (like we do for the port_db script).
H-Shay commentedon Oct 5, 2021
Okay excellent. I will write a test case to keep an eye on this functionality going forwards, fix this issue, and hopefully fix anything else that might have broken in the interval.