-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add account data to export command #14969
Changes from 2 commits
b8fca46
9811c51
bce4861
ab2b030
16e7495
6dfbf5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Adds profile information, devices and connections to the user data export via command line. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ | |
import os | ||
import sys | ||
import tempfile | ||
from typing import List, Optional | ||
from typing import Dict, List, Optional | ||
|
||
from twisted.internet import defer, task | ||
|
||
|
@@ -222,6 +222,32 @@ def write_connections(self, connections: List[JsonDict]) -> None: | |
with open(connection_file, "a") as f: | ||
print(json.dumps(connection), file=f) | ||
|
||
def write_room_account_data( | ||
self, room_id: str, account_data: Dict[str, JsonDict] | ||
) -> None: | ||
account_data_directory = os.path.join( | ||
self.base_directory, "user_data", "account_data" | ||
) | ||
os.makedirs(account_data_directory, exist_ok=True) | ||
|
||
room_file = os.path.join(account_data_directory, room_id) | ||
|
||
with open(room_file, "a") as f: | ||
print(json.dumps(account_data), file=f) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks like its not used anymore? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! I have removed this. |
||
|
||
def write_account_data( | ||
self, file_name: str, account_data: Dict[str, JsonDict] | ||
) -> None: | ||
account_data_directory = os.path.join( | ||
self.base_directory, "user_data", "account_data" | ||
) | ||
os.makedirs(account_data_directory, exist_ok=True) | ||
|
||
account_data_file = os.path.join(account_data_directory, file_name) | ||
|
||
with open(account_data_file, "a") as f: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why append mode here---is this what we typically do when exporting data? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I can see there are some functions that need append, because e.g. room events are added. Here in this specific case not. Ican check in each function if append or write is needed, if it makes a difference. In any case the export starts with an empty directory. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't worry: I think it's okay as-is. I just wanted to understand what was going on (e.g. if we are expected to append to a previous data export). |
||
print(json.dumps(account_data), file=f) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is fine here (we don't expect account_data to be large), but for general interest it might be more efficient to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't that make sense for all functions in this file? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably! But we don't need to change it here and now. |
||
|
||
def finished(self) -> str: | ||
return self.base_directory | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder why not do
rooms
>account_data
and then just have auser_data
>account_data
(orglobal_account_data
)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has no special reasons. My thinking was that account data is user related rather than room related. I will change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not anymore. The PR has now been merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It isn't a big deal, just curious what the thinking was!
They're related to both users & rooms, which is annoying in this case. 😄