Skip to content

Handle SIGINT to abort hanging queries in ValidateBuilderExists#2214

Open
sagnik3788 wants to merge 4 commits intobuildpacks:mainfrom
sagnik3788:main
Open

Handle SIGINT to abort hanging queries in ValidateBuilderExists#2214
sagnik3788 wants to merge 4 commits intobuildpacks:mainfrom
sagnik3788:main

Conversation

@sagnik3788
Copy link

@sagnik3788 sagnik3788 commented Jul 14, 2024

Summary

Fix ValidateBuilderExists function to handle SIGINT signals. This update ensures that interrupted hanging queries gracefully abort, returning an "operation aborted" error

Before

Crtl+C does not abort hanging query

After

operation aborted

Resolves #1242

@sagnik3788 sagnik3788 requested review from a team as code owners July 14, 2024 18:39
@github-actions github-actions bot added the type/enhancement Issue that requests a new feature or improvement. label Jul 14, 2024
@github-actions github-actions bot added this to the 0.35.0 milestone Jul 14, 2024
@sagnik3788
Handle SIGINT to abort hanging queries in ValidateBuilderExists
8e92ddd 
Signed-off-by: sagnik3788 <sagnikdas5432@gmail.com>
@sagnik3788
update
909776c 
Signed-off-by: sagnik3788 <sagnikdas5432@gmail.com>
@sagnik3788
fix errors
101a48f 
Signed-off-by: sagnik3788 <sagnikdas5432@gmail.com>
@sagnik3788
update
c0ee987 
Signed-off-by: sagnik3788 <sagnikdas5432@gmail.com>
@sagnik3788
Copy link
Author

sagnik3788 commented Jul 15, 2024

@natalieparellano, any idea why windows-lcow and windows-wcow are failing at [Makefile:99: unit], while locally make unit passes all the cases?

@natalieparellano
Copy link
Member

natalieparellano commented Jul 15, 2024

@sagnik3788 I'm seeing TestConfigDefaultBuilder/ConfigDefaultBuilderCommand/#ConfigDefaultBuilder/set/SIGINT_is_received_during_query/aborts_the_operation config_default_builder_test.go:223: Expected nil: not supported by windows - I'm guessing you are developing on a Mac or Linux machine?

@sagnik3788
Copy link
Author

sagnik3788 commented Jul 15, 2024 •
edited
Loading

Linux machine @natalieparellano

@natalieparellano natalieparellano modified the milestones: 0.35.0, 0.36.0 Jul 17, 2024
natalieparellano
natalieparellano reviewed Jul 24, 2024
View reviewed changes
internal/commands/config_default_builder.go
Comment on lines +76 to +77
sigChan := make(chan os.Signal, 1)
signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM)
Copy link
Member

@natalieparellano natalieparellano Jul 24, 2024

Choose a reason for hiding this comment

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

We'll probably want a separate implementation of this for linux/darwin and windows. This can be accomplished with separate files, such as signal_unix.go and signal_windows.go, and build tags as shown here.

Copy link
Member

@jjbustamante jjbustamante Sep 19, 2024

Choose a reason for hiding this comment

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

+1 with this comment

@natalieparellano natalieparellano modified the milestones: 0.36.0, 0.37.0 Nov 12, 2024
@jjbustamante
Copy link
Member

jjbustamante commented Dec 4, 2024

Hi @sagnik3788

Do you have some time to address this comment from Natalie?

@jjbustamante jjbustamante modified the milestones: 0.37.0, 0.38.0 Mar 14, 2025
@jjbustamante jjbustamante modified the milestones: 0.38.0, 0.39.0 Jun 3, 2025
@jjbustamante jjbustamante modified the milestones: 0.39.0, 0.40.0 Nov 23, 2025
@jjbustamante jjbustamante modified the milestones: 0.40.0, 0.41.0 Feb 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jjbustamante jjbustamante jjbustamante left review comments

@natalieparellano natalieparellano natalieparellano left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

type/enhancement Issue that requests a new feature or improvement.

Projects

None yet

Milestone

0.41.0

Development

Successfully merging this pull request may close these issues.

setting remote config doesn't respect Ctrl+C

3 participants

@sagnik3788 @natalieparellano @jjbustamante

Comments