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

Change task from being canceled to waiting #118

Merged
merged 11 commits into from
Apr 16, 2024
Merged

Conversation

rlauuzo
Copy link
Collaborator

@rlauuzo rlauuzo commented Apr 14, 2024

Tasks now wait for their turn instead of being canceled

Tasks now wait for their turn instead of being canceled
@AbandonedCart

This comment was marked as off-topic.

@rlauuzo
Copy link
Collaborator Author

rlauuzo commented Apr 14, 2024

Makes sense! Is that easy to do?

@AbandonedCart

This comment was marked as off-topic.

@rlauuzo
Copy link
Collaborator Author

rlauuzo commented Apr 16, 2024

Hey, could someone please test if everything is functioning as intended? Scheduled task now properly waits for an already running task instead of immediately canceling. However, I'm unable to test the automatic tasks due Jellyfin unstable preventing me from changing the plugin settings for some reason.

@rlauuzo
Copy link
Collaborator Author

rlauuzo commented Apr 16, 2024

It seems like this commit might have caused some issues with the settings page? 2442e57

@AbandonedCart
Copy link
Collaborator

Ah, i fixed it on 10.8 and forgot to for master

2442e57#diff-74a94e15e75d4263ca926b506c00750697cbed4a718b367f479a3dd7779510bfR150-R153

MaximumEpisodeCreditsDuration should be MaximumCreditsDuration. The plain text items got missed in the replace.

@AbandonedCart
Copy link
Collaborator

Did you want to throw it into what you're doing or do you want me to commit the change?

@rlauuzo
Copy link
Collaborator Author

rlauuzo commented Apr 16, 2024

I've integrated it, and it seems to be working fine now.

@AbandonedCart
Copy link
Collaborator

Sorry about that. I had done it in the middle of my PR and forgot all about it

@AbandonedCart
Copy link
Collaborator

I have a feeling that replacing my attempt to add items to the queue with an item specific job added to your job queue will fix world hunger.

@rlauuzo
Copy link
Collaborator Author

rlauuzo commented Apr 16, 2024

Haha, let's hope so! It does seem to be functioning as intended now.

@rlauuzo rlauuzo merged commit ad3e0c4 into jumoog:master Apr 16, 2024
3 checks passed
AbandonedCart pushed a commit to RepoDevil/intro-skipper that referenced this pull request Apr 16, 2024
@rlauuzo
Copy link
Collaborator Author

rlauuzo commented Apr 16, 2024

@AbandonedCart We still need to replace Plugin.Instance!.AnalyzerTaskIsRunning, but I'm uncertain if you're checking for scheduled tasks or automatic tasks. ScheduledTaskSemaphore.CurrentCount == 0 indicates a scheduled task running, while Entrypoint.AutomaticTaskState == TaskState.Running signifies an automatic task.

@AbandonedCart
Copy link
Collaborator

A little of both. I was checking for automatic tasks to decide if it should append an item. The scheduled tasks were canceling the automatic.

@rlauuzo
Copy link
Collaborator Author

rlauuzo commented Apr 16, 2024

Does that imply we can also eliminate the _analyzeAgain logic?

@AbandonedCart
Copy link
Collaborator

AbandonedCart commented Apr 16, 2024

That's the goal, but that's currently the backup plan for when items don't make it in time.

What I don't have yet is a way to check the queue for updates before ending the task and then close off accepting additions as soon as the check comes back empty

@AbandonedCart
Copy link
Collaborator

AbandonedCart commented Apr 16, 2024

It may be better to have a single file analyzer and keep adding the file added and file changed to that if it doesn't create a bottleneck.

My idea is that files added or modified work from a queue of individual file scans. When a library gets refreshed, it adds anything not already in the queue to the end. When a scheduled scan gets run, it can cancel everything else. Does that sound right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants