Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Status change on user action #73

Open
wants to merge 35 commits into
base: develop
Choose a base branch
from

Conversation

timsergeeff
Copy link
Contributor

App start status change

Now we have ability to change status on users starting app action.

1-ezgif com-video-to-gif-converter

Pause other tasks with WIP status

We can turn off pausing all other WIP tasks. This logic made of state: only one task can be WIP at a time.

2-ezgif com-video-to-gif-converter

Farm render status change

Previosly task was getting status change (WFA) only at publishing state in deadline so in between of publishing job to farm and publishing to kitsu we have nothing indicating current task status. Now we have FARM status change when farm rendering is on

3-ezgif com-video-to-gif-converter

Additional info

The settings allow to configure status change conditions so statuses will not double. And some other changes to settings needed to be done to make all logic work and ahve opportunity to swith in on or off
image

Testing notes:

  1. Add nessesary statuses to your kitsu production
  2. configure status changes in addon
  3. mark some statuses in kitsu as WIP (only for test)
  4. start other task
  5. see all other tasks now was PAUSED
  6. publish a job on farm
  7. see status change to FARM status

@timsergeeff timsergeeff changed the title Stsus change on user action Status change on user action Sep 6, 2024
@timsergeeff
Copy link
Contributor Author

timsergeeff commented Sep 6, 2024

I have doubts about code style and some decisions i've made to make it work, bot it works now help me please to make code and logic pretty please) Hope you like it!

@MustafaJafar MustafaJafar added type: enhancement Improvement of existing functionality or minor addition community Issues and PRs coming from the community members labels Sep 9, 2024
@MustafaJafar
Copy link
Contributor

could you create an issue for your enhancement request/suggestion and link it to this PR ?
https://github.com/ynput/ayon-kitsu/issues/new/choose

Copy link
Contributor

@MustafaJafar MustafaJafar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't test run the PR yet.
I'd recommend adding settings in multiple places. so that we have them near to each other so that we know that these settings belongs to the same feature.

client/ayon_kitsu/hooks/pre_status_cahnge.py Outdated Show resolved Hide resolved
client/ayon_kitsu/hooks/pre_status_cahnge.py Outdated Show resolved Hide resolved
client/ayon_kitsu/version.py Outdated Show resolved Hide resolved
package.py Outdated Show resolved Hide resolved
Copy link

@BigRoy BigRoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timsergeeff could you please install a linter for your IDE that matches the code style of the repo, mostly PEP08. There's a lot of invalid blank lines.

Also, a spell-checker may also help since the log messages contain typos.

client/ayon_kitsu/hooks/pre_status_cahnge.py Outdated Show resolved Hide resolved
client/ayon_kitsu/hooks/pre_status_cahnge.py Outdated Show resolved Hide resolved
client/ayon_kitsu/hooks/pre_status_cahnge.py Outdated Show resolved Hide resolved
client/ayon_kitsu/hooks/pre_status_cahnge.py Outdated Show resolved Hide resolved
client/ayon_kitsu/hooks/pre_status_cahnge.py Outdated Show resolved Hide resolved
client/ayon_kitsu/plugins/publish/integrate_kitsu_note.py Outdated Show resolved Hide resolved
client/ayon_kitsu/plugins/publish/integrate_kitsu_note.py Outdated Show resolved Hide resolved
server/settings/app_start.py Outdated Show resolved Hide resolved
server/settings/app_start.py Outdated Show resolved Hide resolved
server/settings/publish_plugins.py Outdated Show resolved Hide resolved
timsergeeff and others added 15 commits September 9, 2024 15:57
Co-authored-by: Mustafa Jafar <mustafataherzaky@outlook.com>
Co-authored-by: Mustafa Jafar <mustafataherzaky@outlook.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Mustafa Jafar <mustafataherzaky@outlook.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Mustafa Jafar <mustafataherzaky@outlook.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
timsergeeff and others added 3 commits September 9, 2024 16:04
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filename has a typo cahnge -> change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohh sorry will fix

@timsergeeff
Copy link
Contributor Author

i dont undestand how to remove this
image
and from previos PR there was a comment i think from @BigRoy that we may be dont need this
image

client/ayon_kitsu/addon.py Outdated Show resolved Hide resolved
timsergeeff and others added 2 commits September 9, 2024 16:21
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
timsergeeff and others added 3 commits September 9, 2024 16:21
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
server/settings/app_start.py Outdated Show resolved Hide resolved
Comment on lines 84 to 87
if not project_settings["set_pause_status_to_other_tasks"]:
self.log.info(f"Pausing all other tasks with same status disabled.")
else:
self.log.info(f"Pausing all other tasks with same status enabled.")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't understand this use case - why would launching a task pause all other tasks? :/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a ton of situations when artist started a task and then switched to another and mark it wip resulting in two WIP tasks at the same time and it makes tracking of production confusing and harder to make nessesary desigions fast

Copy link

@BigRoy BigRoy Sep 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a ton of situations when artist started a task and then switched to another and mark it wip resulting in two WIP tasks at the same time and it makes tracking of production confusing and harder to make nessesary desigions fast

It's interesting - because on our studio for example it's quite common for someone to do that (usually working on a few smaller tasks at once) and they'd basically open both and sometimes do a bit in one, then some stuff in the other almost simultaneously. For smaller tasks though where usually waiting needs to happen in-between (e.g. test-rendering, etc. and then continuing with something else while the test render is occurring in the background). Both tasks being active then is the logical state.

Anyway... will leave the "production scenarios" reviewing of this to others. @dee-ynput

Also, something tells me we should be setting this statuses TO AYON - and then sync to all the others or whatever so we don't need to duplicate this logic for kitsu, shotgrid, ftrack, etc. but have on entry point in the code to do this - no matter the production tracker integration. But maybe I'm too hopeful.

timsergeeff and others added 5 commits September 9, 2024 16:37
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
@timsergeeff
Copy link
Contributor Author

@MustafaJafar i think i cant connect pr with issue because of rules of repo

@BigRoy BigRoy linked an issue Sep 9, 2024 that may be closed by this pull request
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Issues and PRs coming from the community members type: enhancement Improvement of existing functionality or minor addition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Status change on user action
3 participants