Refactor/Sync statuses via listener & sync statuses by kitsu project settings#130
Refactor/Sync statuses via listener & sync statuses by kitsu project settings#130YuanQI295 wants to merge 8 commits into
Conversation
Sharkitty
left a comment
Sharkitty
left a comment
There was a problem hiding this comment.
Generally speaking, when you do a PR, you should indicate if it's a feature, an enhancement or a fix in the PR title. It's even better if it's reflected in the branch name, and the first commit of your branch. I'm not gonna ask you to rename your branch but for next time, keep that in mind (please add that to the PR name though). Don't forget to lint your code. Besides that it's good, only minor things that need to be enhanced imo.
|
|
||
| async def parse_statuses( | ||
| addon: "KitsuAddon", kitsu_project_id: str | ||
| addon: "KitsuAddon", kitsu_project_id: str, ayon_project: ProjectEntity | None = None, |
There was a problem hiding this comment.
This line is too long. In the Ynput repositories, the maximum line length is 79.
| """ | ||
|
|
||
| task_status_response = await addon.kitsu.get("data/task-status") | ||
| # Get the statuses from the project instead of studio |
There was a problem hiding this comment.
Comments should say what your code do, not how you changed the code. If that makes sense? So here you could say it's getting task statuses from project settings, but you should omit that the former implementation was using studio settings.
|
|
||
| task_status_response = await addon.kitsu.get("data/task-status") | ||
| # Get the statuses from the project instead of studio | ||
| task_status_response = await addon.kitsu.get(f"data/projects/{kitsu_project_id}/settings/task-status") |
There was a problem hiding this comment.
Line too long and trailing whitespace.
|
|
||
| # Ensure that status list gets every linked Ayon statuses | ||
| if ayon_project: | ||
| kitsu_statuses_names = {s.name for s in result} |
There was a problem hiding this comment.
only put names in plural, not statuses in that case.
There was a problem hiding this comment.
status doesn't take an e in singular
| # Ensure that status list gets every linked Ayon statuses | ||
| if ayon_project: | ||
| kitsu_statuses_names = {s.name for s in result} | ||
| for existing_status in ayon_project.statuses: |
There was a problem hiding this comment.
I think status would be clear enough as a variable name here.
| "Person", | ||
| "Project", | ||
| "SyncCasting", | ||
| "TaskStatus" |
There was a problem hiding this comment.
It's best to add a comma , here so next time someone needs to work on this file, they don't need to edit this line to add a new entry.
| entity_dict: "EntityDict", | ||
| mock: bool = False, | ||
| ): | ||
| logging.info("sync_project") |
There was a problem hiding this comment.
Why are you removing logs that are not related to your changes?
| elif entity_dict["type"] == "TaskStatus": | ||
| if project: |
There was a problem hiding this comment.
simpler: elif and project
| project_name (str): The Ayon | ||
| """ | ||
| start_time = time.time() | ||
| logging.info(f"Syncing kitsu project {kitsu_project_id} to {project_name}") |
There was a problem hiding this comment.
No need to remove this either.
|
One thing I don't really understand however, is why the sync now button requires the listener to be turned off? If I understand correctly, an admin would have to go out of their way to disable it before using this feature. And I think that would be a problem. |
I thought the sync now button could stay a security. Per exemple if the processor stopped for a moment, during that time Ayon would not receive any update. We could use the button to catch up ? |
Yeah but why should the processor be stopped manually for this process? |
I'm not sure I fully understand your question. If the listener is not stopped, the statuses will automatically be added to Ayon, so to simulate a malfunction we could stop the listener? |
Ah, so this is just something you need to do for testing then? |
YuanQI295
changed the title
yes |
2 participants
Changelog Description
At the pairing of a project from Kitsu to Ayon, all studio statuses were imported into Ayon. This PR imports only the statuses attributed to that specific project. It also syncs any new status added to the Kitsu project in real time, the status appears in Ayon immediately after being added in Kitsu.
Additional review information
The adding of statuses in real time goes through the event called
project:update. To test theSync nowbutton (the safety net in case the processor doesn't sync statuses properly), you need to stop the listener for that event.Testing notes:
Task Statussettings, assign some statuses that are specific to that project.Pair projectin the Kitsu menu.project:updatelistener inprocessor.py.Sync nowbutton in the Kitsu menu and verify the new status appears in Ayon.