This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git
The following commit(s) were added to refs/heads/main by this push:
new cc999d182 refactor(config)!: use one error-returning config loader
(#2294)
cc999d182 is described below
commit cc999d18279e817024ad24656e011ae880c9e551
Author: Minh Vu <[email protected]>
AuthorDate: Mon Oct 26 16:51:09 2026 +0100
refactor(config)!: use one error-returning config loader (#2294)
* refactor(config)!: use one error-returning config loader
Signed-off-by: Minh Vu <[email protected]>
* chore: sync release notes with main to resolve conflicts
---------
Signed-off-by: Minh Vu <[email protected]>
---
cmd/iceberg/main.go | 2 +-
config/config.go | 23 ++++++++---------
config/config_test.go | 71 ++++++++++++++++++++++++++++++++++++++++++++++++---
3 files changed, 78 insertions(+), 18 deletions(-)
diff --git a/cmd/iceberg/main.go b/cmd/iceberg/main.go
index 47adb4a34..d18b2fc77 100644
--- a/cmd/iceberg/main.go
+++ b/cmd/iceberg/main.go
@@ -868,7 +868,7 @@ func resolveCatalogName(explicitFlags map[string]bool,
flagValue string) string
}
func applyConfigFile(args *Args, explicitFlags map[string]bool) error {
- configData, err := config.LoadConfigFile(args.Config)
+ configData, err := config.LoadConfig(args.Config)
if err != nil {
return err
}
diff --git a/config/config.go b/config/config.go
index 7cf92859a..6a6bb53c7 100644
--- a/config/config.go
+++ b/config/config.go
@@ -55,18 +55,12 @@ type CatalogConfig struct {
RestOptions *RestOptions `yaml:"rest,omitempty"`
}
-// LoadConfig returns configuration data when it can be read. It retains the
-// historical best-effort behavior; callers that must distinguish a missing
-// default from a broken explicit file should use LoadConfigFile.
-func LoadConfig(configPath string) []byte {
- data, _ := LoadConfigFile(configPath)
-
- return data
-}
-
-// LoadConfigFile loads configuration data and reports path and read failures.
-// A missing implicit home-directory configuration is not an error.
-func LoadConfigFile(configPath string) ([]byte, error) {
+// LoadConfig reads configuration data and reports path and read failures.
+// An empty configPath selects ~/.iceberg-go.yaml. A missing implicit
+// home-directory configuration is not an error; LoadConfig returns nil data.
+// Unlike [EnvConfig], it ignores GOICEBERG_HOME. Use [ParseConfig] to decode
+// the YAML and select a catalog.
+func LoadConfig(configPath string) ([]byte, error) {
implicit := configPath == ""
var path string
if !implicit {
@@ -132,7 +126,10 @@ func fromConfigFiles() Config {
}
var cfg Config
- if err := yaml.Unmarshal(LoadConfig(dir), &cfg); err != nil {
+ // EnvConfig keeps its historical best-effort initialization behavior.
+ // Callers reading an explicit file should handle errors from
LoadConfig.
+ data, _ := LoadConfig(dir)
+ if err := yaml.Unmarshal(data, &cfg); err != nil {
return cfg
}
diff --git a/config/config_test.go b/config/config_test.go
index 40da5a6a9..a335a07fa 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -26,17 +26,69 @@ import (
"github.com/stretchr/testify/require"
)
-func TestLoadConfigFile(t *testing.T) {
+func TestLoadConfig(t *testing.T) {
+ t.Run("explicit file contents", func(t *testing.T) {
+ path := filepath.Join(t.TempDir(), "config.yaml")
+ contents := []byte("default-catalog: test\n")
+ require.NoError(t, os.WriteFile(path, contents, 0o600))
+
+ data, err := LoadConfig(path)
+ require.NoError(t, err)
+ assert.Equal(t, contents, data)
+ })
+
+ t.Run("explicit relative config path", func(t *testing.T) {
+ t.Chdir(t.TempDir())
+ contents := []byte("max-workers: 7\n")
+ require.NoError(t, os.WriteFile("relative.yaml", contents,
0o600))
+
+ data, err := LoadConfig("relative.yaml")
+ require.NoError(t, err)
+ assert.Equal(t, contents, data)
+ })
+
+ t.Run("implicit home directory config", func(t *testing.T) {
+ home := t.TempDir()
+ t.Setenv("HOME", home)
+ contents := []byte("max-workers: 3\n")
+ require.NoError(t, os.WriteFile(filepath.Join(home, cfgFile),
contents, 0o600))
+
+ data, err := LoadConfig("")
+ require.NoError(t, err)
+ assert.Equal(t, contents, data)
+ })
+
t.Run("implicit missing config", func(t *testing.T) {
t.Setenv("HOME", t.TempDir())
- data, err := LoadConfigFile("")
+ data, err := LoadConfig("")
require.NoError(t, err)
assert.Nil(t, data)
})
+ t.Run("implicit config ignores GOICEBERG_HOME", func(t *testing.T) {
+ t.Setenv("HOME", t.TempDir())
+ alternateHome := t.TempDir()
+ t.Setenv("GOICEBERG_HOME", alternateHome)
+ require.NoError(t, os.WriteFile(filepath.Join(alternateHome,
cfgFile), []byte("max-workers: 7\n"), 0o600))
+
+ data, err := LoadConfig("")
+ require.NoError(t, err)
+ assert.Nil(t, data)
+ })
+
+ t.Run("implicit config with unavailable home", func(t *testing.T) {
+ t.Setenv("HOME", "")
+ if _, err := os.UserHomeDir(); err == nil {
+ t.Skip("this platform still resolves a home directory
without HOME")
+ }
+ _, err := LoadConfig("")
+ require.Error(t, err)
+ assert.ErrorContains(t, err, "resolve home directory")
+ })
+
t.Run("explicit missing config", func(t *testing.T) {
path := filepath.Join(t.TempDir(), "missing.yaml")
- _, err := LoadConfigFile(path)
+ _, err := LoadConfig(path)
require.Error(t, err)
assert.ErrorIs(t, err, os.ErrNotExist)
assert.ErrorContains(t, err, path)
@@ -44,12 +96,23 @@ func TestLoadConfigFile(t *testing.T) {
t.Run("explicit path is directory", func(t *testing.T) {
path := t.TempDir()
- _, err := LoadConfigFile(path)
+ _, err := LoadConfig(path)
require.Error(t, err)
assert.ErrorContains(t, err, path)
})
}
+// An unreadable explicit file remains best-effort for the EnvConfig helper.
+func TestFromConfigFilesIgnoresReadErrors(t *testing.T) {
+ configHome := t.TempDir()
+ t.Setenv("GOICEBERG_HOME", configHome)
+ require.NoError(t, os.Mkdir(filepath.Join(configHome, cfgFile), 0o700))
+
+ cfg := fromConfigFiles()
+ assert.Equal(t, "default", cfg.DefaultCatalog)
+ assert.Equal(t, defaultMaxWorkers, cfg.MaxWorkers)
+}
+
var testArgs = []struct {
file []byte
catName string
