Smart syncing of Users#1165
Conversation
…tion Added method to recursively GET users if paginated and another method to parse URL and return endpoint and query parameters.
|
This PR will be ready for review when the related site PR has been merged. |
instead of creating and updating a single user at a time, a list of dicts will be sent for bulk update and creation.
RohanJnr
force-pushed
the
smart_syncing_users
branch
from
5514d03 to
eca87e3
Compare
… and the modified user data.
RohanJnr
requested review from
ikuyarihS and
jb3
and removed request for
a team
ks129
left a comment
ks129
left a comment
There was a problem hiding this comment.
Currently some code style/small changes, but there is one big issue: Type hinting variables that have very clear type is pointless.
| return users | ||
|
|
||
| @staticmethod | ||
| def get_endpoint(url: str) -> tuple: |
There was a problem hiding this comment.
I think it's better to use typing.Tuple for this.
| for user in diff.updated: | ||
| await self.bot.api_client.put(f'bot/users/{user.id}', json=user._asdict()) | ||
| if diff.created: | ||
| created: list = [user._asdict() for user in diff.created] |
There was a problem hiding this comment.
This have already [] and this indicates that this is list. I think type hint in this variable is unnecessary.
| created: list = [user._asdict() for user in diff.created] | ||
| await self.bot.api_client.post("bot/users", json=created) | ||
| if diff.updated: | ||
| updated: list = [self.patch_dict(user) for user in diff.updated] |
| if diff.created: | ||
| created: list = [user._asdict() for user in diff.created] | ||
| await self.bot.api_client.post("bot/users", json=created) | ||
| if diff.updated: |
There was a problem hiding this comment.
Looks like you removed log of users updating. I think it's better to restore it.
| @staticmethod | ||
| def patch_dict(user: _User) -> dict: | ||
| """Convert namedtuple to dict by omitting None values.""" | ||
| user_dict: dict = {} |
There was a problem hiding this comment.
{} already indicates that this is dict, no need for type hint.
| return endpoint, params | ||
|
|
||
| @staticmethod | ||
| def patch_dict(user: _User) -> dict: |
There was a problem hiding this comment.
Use typing.Dictand provide key-value too.
ghost
added
s: waiting for author
ghost
added
needs 2 approvals
and removed
s: waiting for author
MarkKoz
added
p: 1 - high
ghost
added
s: waiting for author
ghost
added
needs 2 approvals
and removed
s: waiting for author
…into smart_syncing_users
ghost
added
s: waiting for author
There was a problem hiding this comment.
Almost forgot: the code in my comment omitted the comments only for the sake of example. I think the comments that existed in _get_diff were useful and you should keep them in one form or another. They may not make 100% sense any more due to changes, so you may need to revise them a bit.
ghost
added
needs 2 approvals
and removed
s: waiting for author
3 participants
Closes #1119
Implementation
Details
This PR is directly linked with the Site repo PR Add pagination and bulk update support for User(discord user) serializer. site#378
This PR uses the changes made in the above-mentioned PR.
Only merge this PR after the Site PR is merged.