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.

Sign up for GitHub

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

Jump to bottom

Fix incorrect state after indicator cancellation #346

Merged
merged 2 commits into from
Oct 10, 2024
Merged

Fix incorrect state after indicator cancellation #346

merged 2 commits into from
Oct 10, 2024

Conversation

arksap2002
Copy link
Collaborator

@arksap2002 arksap2002 commented Sep 3, 2024 •
edited by Vladislav0Art
Loading

Description of changes made

isProcessStopped method calls relocation due to long time methods processing

TODOs

  • Publish core module of version 5.0.1.

Other notes

Closes #213 #314

  • I have checked that I am merging into correct branch

@arksap2002 arksap2002 added bug Something isn't working Ready for review PR redy for review labels Sep 3, 2024
@arksap2002 arksap2002 changed the title fix incorrect state after indicator cancellation Fix incorrect state after indicator cancellation Sep 3, 2024
@arksap2002 arksap2002 added Ready for review PR redy for review and removed Ready for review PR redy for review labels Sep 3, 2024
@arksap2002
Copy link
Collaborator Author

Similar to #216

pderakhshanfar
pderakhshanfar approved these changes Sep 18, 2024
View reviewed changes
Copy link
Collaborator

@pderakhshanfar pderakhshanfar left a comment

Choose a reason for hiding this comment

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

For now, it is fine, but in the future, we need to find a better way to handle cancelation instead of repeating isCanceled check after each statement

@pderakhshanfar pderakhshanfar added the In progress PR is in progress label Sep 18, 2024
@arksap2002 arksap2002 linked an issue Sep 18, 2024 that may be closed by this pull request
stop() should be called only if start() called before #314
Closed
@Vladislav0Art
Copy link
Collaborator

How do you ensure that #314 gets closed?
@arksap2002

@Vladislav0Art
Copy link
Collaborator

For now, it is fine, but in the future, we need to find a better way to handle cancelation instead of repeating isCanceled check after each statement

Kotlin coroutines support some smart cancelation mechanisms; transitioning to them in the future should help with this.

Vladislav0Art
Vladislav0Art approved these changes Oct 2, 2024
View reviewed changes
Copy link
Collaborator

@Vladislav0Art Vladislav0Art left a comment

Choose a reason for hiding this comment

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

Before merging the PR, please answer this question.
@arksap2002

@Vladislav0Art Vladislav0Art removed the In progress PR is in progress label Oct 7, 2024
@arksap2002
Copy link
Collaborator Author

replay to #346 (comment)
@Vladislav0Art I tested cancellation of all staes of test generation, I don't have this error anymore

@arksap2002
Copy link
Collaborator Author

This PR is ready to merge

@Vladislav0Art
Copy link
Collaborator

@arksap2002 Do not forget to publish a new patch version of the core module. You have made changes to the implementation of FeedbackCycleWithLLM.

arksap2002 and others added 2 commits October 10, 2024 13:22
@arksap2002
fix incorrect state after indicator cancellation
f05e950
publish: core module version 5.0.1
5c7a385 
The patch version changed due to fixes in the internal implementation of the `LLMWithFeedbackCycle.run` method.
@Vladislav0Art Vladislav0Art force-pushed the arksap2002/bugs/incorrect-state-after-indicator-cancellation branch from 6529d29 to 5c7a385 Compare October 10, 2024 11:28
@Vladislav0Art
Copy link
Collaborator

@arksap2002 Do not forget to publish a new patch version of the core module. You have made changes to the implementation of FeedbackCycleWithLLM.

Published core module of version 5.0.1.

@Vladislav0Art Vladislav0Art merged commit 4d61f12 into development Oct 10, 2024
3 checks passed
@arksap2002 arksap2002 deleted the arksap2002/bugs/incorrect-state-after-indicator-cancellation branch October 15, 2024 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pderakhshanfar pderakhshanfar pderakhshanfar approved these changes

@Vladislav0Art Vladislav0Art Vladislav0Art approved these changes

Assignees
No one assigned
Labels
bug Something isn't working Ready for review PR redy for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

stop() should be called only if start() called before Incorrect state after indicator cancellation
3 participants
@arksap2002 @Vladislav0Art @pderakhshanfar