Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
ashmeenkaur committed Apr 26, 2024
1 parent 50f023a commit 120331b
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 6 deletions.
2 changes: 1 addition & 1 deletion flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func newApp() (app *cli.App) {
cli.BoolFlag{
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.",
"This prevents those signals from immediately terminating gcsfuse inflight operations.",
},

/////////////////////////
Expand Down
4 changes: 3 additions & 1 deletion internal/config/config_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,16 @@ func OverrideWithLoggingFlags(mountConfig *MountConfig, logFile string, logForma
}
}

// cliContext is abstraction over the IsSet() method of cli.Context, Specially
// added to keep OverrideWithIgnoreInterruptsFlag method's unit test simple.
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(c cliContext, mountConfig *MountConfig, ignoreInterruptsFlag bool) {
// If the ignore-interrupts Flag is set, give it priority over the value in config file.
// 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
2 changes: 1 addition & 1 deletion internal/config/config_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func TestOverrideWithIgnoreInterruptsFlag(t *testing.T) {

OverrideWithIgnoreInterruptsFlag(testContext, mountConfig, tt.ignoreInterruptFlagValue)

AssertEq(mountConfig.FileSystemConfig.IgnoreInterrupts, tt.expectedIgnoreInterrupt)
AssertEq(tt.expectedIgnoreInterrupt, mountConfig.FileSystemConfig.IgnoreInterrupts)
})
}
}
2 changes: 1 addition & 1 deletion internal/config/mount_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ type EnableHNS bool
type CacheDir string

type FileSystemConfig struct {
IgnoreInterrupts bool `yaml:"ignore-interrupts,omitempty"`
IgnoreInterrupts bool `yaml:"ignore-interrupts"`
}

type FileCacheConfig struct {
Expand Down
4 changes: 2 additions & 2 deletions internal/config/yaml_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,14 +233,14 @@ func (t *YamlParserTest) TestReadConfigFile_GrpcClientConfig_unsetConnPoolSize()
AssertEq(DefaultGrpcConnPoolSize, mountConfig.GrpcClientConfig.ConnPoolSize)
}

func (t *YamlParserTest) TestReadConfigFile_FileSystemConfig_invalidIgnoreInterruptsValue() {
func (t *YamlParserTest) TestReadConfigFile_FileSystemConfig_InvalidIgnoreInterruptsValue() {
_, err := ParseConfigFile("testdata/file_system_config/invalid_ignore_interrupts.yaml")

AssertNe(nil, err)
AssertTrue(strings.Contains(err.Error(), "error parsing config file: yaml: unmarshal errors:\n line 2: cannot unmarshal !!str `abc` into bool"))
}

func (t *YamlParserTest) TestReadConfigFile_FileSystemConfig_unsetIgnoreInterruptsValue() {
func (t *YamlParserTest) TestReadConfigFile_FileSystemConfig_UnsetIgnoreInterruptsValue() {
mountConfig, err := ParseConfigFile("testdata/file_system_config/unset_ignore_interrupts.yaml")

AssertEq(nil, err)
Expand Down

0 comments on commit 120331b

Please sign in to comment.