Skip to content

Commit

Permalink
changes to pick up flag value on when it is set
Browse files Browse the repository at this point in the history
  • Loading branch information
ashmeenkaur committed Apr 24, 2024
1 parent 91d4011 commit 68f57ea
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 26 deletions.
4 changes: 2 additions & 2 deletions flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func newApp() (app *cli.App) {
},

cli.BoolFlag{
Name: "ignore-interrupts",
Name: config.IgnoreInterruptsFlagName,
Usage: "Instructs gcsfuse to ignore system interrupt signals (like SIGINT, triggered by Ctrl+C). " +
"This prevents those signals from immediately terminating gcsfuse operations in progress.",
},
Expand Down Expand Up @@ -513,7 +513,7 @@ func populateFlags(c *cli.Context) (flags *flagStorage, err error) {
ImplicitDirs: c.Bool("implicit-dirs"),
OnlyDir: c.String("only-dir"),
RenameDirLimit: int64(c.Int("rename-dir-limit")),
IgnoreInterrupts: c.Bool("ignore-interrupts"),
IgnoreInterrupts: c.Bool(config.IgnoreInterruptsFlagName),

// GCS,
CustomEndpoint: customEndpoint,
Expand Down
12 changes: 9 additions & 3 deletions internal/config/config_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

package config

const IgnoreInterruptsFlagName = "ignore-interrupts"

// OverrideWithLoggingFlags overwrites the configs with the flag values if the
// config values are empty.
func OverrideWithLoggingFlags(mountConfig *MountConfig, logFile string, logFormat string,
Expand All @@ -33,11 +35,15 @@ func OverrideWithLoggingFlags(mountConfig *MountConfig, logFile string, logForma
}
}

type cliContext interface {
IsSet(string) bool
}

// OverrideWithIgnoreInterruptsFlag overwrites the ignore-interrupts config with
// the ignore-interrupts flag value if the flag is set.
func OverrideWithIgnoreInterruptsFlag(mountConfig *MountConfig, ignoreInterruptsFlag bool) {
// If the ignoreInterruptsFlag is set, give it priority over the value in config file.
if ignoreInterruptsFlag {
func OverrideWithIgnoreInterruptsFlag(c cliContext, mountConfig *MountConfig, ignoreInterruptsFlag bool) {
// If the ignore-interrupts Flag is set, give it priority over the value in config file.
if c.IsSet(IgnoreInterruptsFlagName) {
mountConfig.FileSystemConfig.IgnoreInterrupts = ignoreInterruptsFlag
}
}
Expand Down
48 changes: 28 additions & 20 deletions internal/config/config_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package config

import (
"log"
"testing"

. "github.com/jacobsa/ogletest"
Expand Down Expand Up @@ -120,29 +119,38 @@ func (t *ConfigTest) TestIsFileCacheEnabled() {
AssertEq(IsFileCacheEnabled(mountConfig3), false)
}

func (t *ConfigTest) TestOverrideWithIgnoreInterruptsFlag() {
type TestCliContext struct {
isSet bool
}

func (s *TestCliContext) IsSet(flag string) bool {
return s.isSet
}

func TestOverrideWithIgnoreInterruptsFlag(t *testing.T) {
var overrideWithIgnoreInterruptsFlagTests = []struct {
testName string
fileSystemConfig FileSystemConfig
testFlags flags
expectedIgnoreInterruptsConfig bool
testName string
ignoreInterruptConfigValue bool
isFlagSet bool
ignoreInterruptFlagValue bool
expectedIgnoreInterrupt bool
}{
{"file system config empty and flag empty", FileSystemConfig{}, flags{}, false},
{"file system config empty and ignore-interrupts flag false", FileSystemConfig{}, flags{IgnoreInterrupts: false}, false},
{"file system config empty and ignore-interrupts flag false", FileSystemConfig{}, flags{IgnoreInterrupts: true}, true},
{"ignore-interrupts config true and flag empty", FileSystemConfig{IgnoreInterrupts: true}, flags{}, true},
{"ignore-interrupts config false and flag empty", FileSystemConfig{IgnoreInterrupts: false}, flags{}, false},
{"ignore-interrupts config false and ignore-interrupts flag false", FileSystemConfig{IgnoreInterrupts: false}, flags{IgnoreInterrupts: false}, false},
{"ignore-interrupts config false and ignore-interrupts flag true", FileSystemConfig{IgnoreInterrupts: false}, flags{IgnoreInterrupts: true}, true},
{"ignore-interrupts config true and ignore-interrupts flag false", FileSystemConfig{IgnoreInterrupts: true}, flags{IgnoreInterrupts: false}, true},
{"ignore-interrupts config true and ignore-interrupts flag true", FileSystemConfig{IgnoreInterrupts: true}, flags{IgnoreInterrupts: true}, true},
{"ignore-interrupts config true and flag not set", true, false, false, true},
{"ignore-interrupts config false and flag not set", false, false, false, false},
{"ignore-interrupts config false and ignore-interrupts flag false", false, true, false, false},
{"ignore-interrupts config false and ignore-interrupts flag true", false, true, true, true},
{"ignore-interrupts config true and ignore-interrupts flag false", true, true, false, false},
{"ignore-interrupts config true and ignore-interrupts flag true", true, true, true, true},
}

for _, tt := range overrideWithIgnoreInterruptsFlagTests {
log.Print("Running:" + tt.testName)
mountConfig := &MountConfig{}
mountConfig.FileSystemConfig = tt.fileSystemConfig
OverrideWithIgnoreInterruptsFlag(mountConfig, tt.testFlags.IgnoreInterrupts)
AssertEq(mountConfig.FileSystemConfig.IgnoreInterrupts, tt.expectedIgnoreInterruptsConfig)
t.Run(tt.testName, func(t *testing.T) {
testContext := &TestCliContext{isSet: tt.isFlagSet}
mountConfig := &MountConfig{FileSystemConfig: FileSystemConfig{IgnoreInterrupts: tt.ignoreInterruptConfigValue}}

OverrideWithIgnoreInterruptsFlag(testContext, mountConfig, tt.ignoreInterruptFlagValue)

AssertEq(mountConfig.FileSystemConfig.IgnoreInterrupts, tt.expectedIgnoreInterrupt)
})
}
}
2 changes: 1 addition & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func runCLIApp(c *cli.Context) (err error) {

config.OverrideWithLoggingFlags(mountConfig, flags.LogFile, flags.LogFormat,
flags.DebugFuse, flags.DebugGCS, flags.DebugMutex)

config.OverrideWithIgnoreInterruptsFlag(c, mountConfig, flags.IgnoreInterrupts)
// Ideally this call to SetLogFormat (which internally creates a new defaultLogger)
// should be set as an else to the 'if flags.Foreground' check below, but currently
// that means the logs generated by resolveConfigFilePaths below don't honour
Expand Down

0 comments on commit 68f57ea

Please sign in to comment.