Skip to content

Commit 49614f5

Browse files
jachym-tousek-keboolajachym-tousek-keboola
committed
refactor: Reduce nesting
1 parent 09e9666 commit 49614f5

21 files changed

Lines changed: 774 additions & 663 deletions

File tree

‎internal/pkg/mapper/datagateway/remote_load.go‎

Lines changed: 46 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -63,52 +63,55 @@ func (m *dataGatewayMapper) AfterRemoteOperation(ctx context.Context, changes *m
6363

6464
// Check if workspace was created and details were set
6565
workspaceID, found, _ = config.Content.GetNested("parameters.db.workspaceId")
66-
if found && workspaceID != nil && workspaceID != "" {
67-
// Update the configuration in remote with workspace details
68-
// If this update fails, the workspace exists but the config doesn't reference it.
69-
// This creates an inconsistent state, so we return the error to prevent silent failures.
70-
api := m.KeboolaProjectAPI()
71-
changedFields := model.NewChangedFields()
72-
changedFields.Add("configuration")
73-
apiObject, apiChangedFields := config.ToAPIObject("Workspace created and configuration updated", changedFields)
74-
_, err := api.UpdateRequest(apiObject, apiChangedFields).Send(ctx)
75-
if err != nil {
76-
// Return error instead of logging and continuing.
77-
// The workspace has been created and should be used, but the config update failed.
78-
// This error must be propagated to prevent inconsistent state.
79-
errs.Append(errors.Errorf(`cannot update configuration "%s" with workspace details: %w`, config.Name, err))
80-
continue
66+
if !found || workspaceID == nil || workspaceID == "" {
67+
continue
68+
}
69+
70+
// Update the configuration in remote with workspace details
71+
// If this update fails, the workspace exists but the config doesn't reference it.
72+
// This creates an inconsistent state, so we return the error to prevent silent failures.
73+
api := m.KeboolaProjectAPI()
74+
changedFields := model.NewChangedFields()
75+
changedFields.Add("configuration")
76+
apiObject, apiChangedFields := config.ToAPIObject("Workspace created and configuration updated", changedFields)
77+
_, err := api.UpdateRequest(apiObject, apiChangedFields).Send(ctx)
78+
if err != nil {
79+
// Return error instead of logging and continuing.
80+
// The workspace has been created and should be used, but the config update failed.
81+
// This error must be propagated to prevent inconsistent state.
82+
errs.Append(errors.Errorf(`cannot update configuration "%s" with workspace details: %w`, config.Name, err))
83+
continue
84+
}
85+
m.logger.Debugf(ctx, `Updated configuration "%s" with workspace details`, config.Name)
86+
87+
// Update remote state with the updated config
88+
configState.SetRemoteState(config)
89+
90+
// Sync workspace details to local state if it exists
91+
if configState.Local == nil {
92+
continue
93+
}
94+
if err := configState.Local.Content.SetNested("parameters.db.workspaceId", workspaceID); err != nil {
95+
m.logger.Warnf(ctx, `Failed to sync workspaceId to local state for config "%s": %s`, config.Name, err.Error())
96+
}
97+
normalizeWorkspaceID(configState.Local)
98+
// Also sync other workspace details
99+
if host, found, _ := config.Content.GetNested("parameters.db.host"); found {
100+
if err := configState.Local.Content.SetNested("parameters.db.host", host); err != nil {
101+
m.logger.Warnf(ctx, `Failed to sync host to local state for config "%s": %s`, config.Name, err.Error())
102+
}
103+
}
104+
if user, found, _ := config.Content.GetNested("parameters.db.user"); found {
105+
if err := configState.Local.Content.SetNested("parameters.db.user", user); err != nil {
106+
m.logger.Warnf(ctx, `Failed to sync user to local state for config "%s": %s`, config.Name, err.Error())
81107
}
82-
m.logger.Debugf(ctx, `Updated configuration "%s" with workspace details`, config.Name)
83-
84-
// Update remote state with the updated config
85-
configState.SetRemoteState(config)
86-
87-
// Sync workspace details to local state if it exists
88-
if configState.Local != nil {
89-
if err := configState.Local.Content.SetNested("parameters.db.workspaceId", workspaceID); err != nil {
90-
m.logger.Warnf(ctx, `Failed to sync workspaceId to local state for config "%s": %s`, config.Name, err.Error())
91-
}
92-
normalizeWorkspaceID(configState.Local)
93-
// Also sync other workspace details
94-
if host, found, _ := config.Content.GetNested("parameters.db.host"); found {
95-
if err := configState.Local.Content.SetNested("parameters.db.host", host); err != nil {
96-
m.logger.Warnf(ctx, `Failed to sync host to local state for config "%s": %s`, config.Name, err.Error())
97-
}
98-
}
99-
if user, found, _ := config.Content.GetNested("parameters.db.user"); found {
100-
if err := configState.Local.Content.SetNested("parameters.db.user", user); err != nil {
101-
m.logger.Warnf(ctx, `Failed to sync user to local state for config "%s": %s`, config.Name, err.Error())
102-
}
103-
}
104-
if database, found, _ := config.Content.GetNested("parameters.db.database"); found {
105-
if err := configState.Local.Content.SetNested("parameters.db.database", database); err != nil {
106-
m.logger.Warnf(ctx, `Failed to sync database to local state for config "%s": %s`, config.Name, err.Error())
107-
}
108-
}
109-
scheduleLocalSave(configState)
108+
}
109+
if database, found, _ := config.Content.GetNested("parameters.db.database"); found {
110+
if err := configState.Local.Content.SetNested("parameters.db.database", database); err != nil {
111+
m.logger.Warnf(ctx, `Failed to sync database to local state for config "%s": %s`, config.Name, err.Error())
110112
}
111113
}
114+
scheduleLocalSave(configState)
112115
}
113116

114117
// Process loaded configs - backfill workspace details

‎internal/pkg/project/manifest/manifest.go‎

Lines changed: 56 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -86,52 +86,9 @@ func Load(ctx context.Context, logger log.Logger, fs filesystem.Fs, envs env.Pro
8686
// Override ProjectID and BranchID by ENVs, if enabled
8787
var mapping []mappingItem
8888
if content.AllowTargetENV {
89-
projectIDStr := envs.Get(ProjectIDOverrideENV)
90-
branchIDStr := envs.Get(BranchIDOverrideENV)
91-
92-
if branchIDStr != "" && len(content.Branches) != 1 {
93-
return nil, errors.Errorf(`env %s=%s can be used if there is one branch in the manifest, found %d branches`, BranchIDOverrideENV, branchIDStr, len(content.Branches))
94-
}
95-
96-
if projectIDStr != "" {
97-
if projectIDInt, err := strconv.Atoi(projectIDStr); err == nil {
98-
projectID := keboola.ProjectID(projectIDInt)
99-
if projectID != content.Project.ID {
100-
logger.Infof(ctx, `Overriding the project ID by the environment variable %s=%v`, ProjectIDOverrideENV, projectID)
101-
mapping = append(mapping, mappingItem{
102-
ManifestValue: content.Project.ID,
103-
MemoryValue: projectID,
104-
})
105-
}
106-
} else {
107-
return nil, errors.Errorf(`env %s=%s is not valid project ID`, ProjectIDOverrideENV, projectIDStr)
108-
}
109-
}
110-
111-
if branchIDStr != "" {
112-
if branchIDInt, err := strconv.Atoi(branchIDStr); err == nil {
113-
originalBranchID := content.Branches[0].ID
114-
replacedBranchID := keboola.BranchID(branchIDInt)
115-
if replacedBranchID != content.Branches[0].ID {
116-
logger.Infof(ctx, `Overriding the branch ID by the environment variable %s=%v`, BranchIDOverrideENV, replacedBranchID)
117-
// Map branch ID in all objects
118-
mapping = append(mapping, mappingItem{
119-
ManifestValue: originalBranchID,
120-
MemoryValue: replacedBranchID,
121-
})
122-
// Map allowed branches filter
123-
mapping = append(mapping, mappingItem{
124-
ManifestValue: model.AllowedBranch(originalBranchID.String()),
125-
MemoryValue: model.AllowedBranch(replacedBranchID.String()),
126-
})
127-
}
128-
// Replace main branch in the filter, with branch ID, if needed
129-
if len(content.AllowedBranches) == 1 && content.AllowedBranches[0] == model.MainBranchDef {
130-
content.AllowedBranches[0] = model.AllowedBranch(replacedBranchID.String())
131-
}
132-
} else {
133-
return nil, errors.Errorf(`env %s=%s is not valid branch ID`, BranchIDOverrideENV, branchIDStr)
134-
}
89+
mapping, err = useIDsFromENV(ctx, logger, envs, content)
90+
if err != nil {
91+
return nil, err
13592
}
13693
}
13794

@@ -167,6 +124,59 @@ func Load(ctx context.Context, logger log.Logger, fs filesystem.Fs, envs env.Pro
167124
return m, nil
168125
}
169126

127+
func useIDsFromENV(ctx context.Context, logger log.Logger, envs env.Provider, content *file) ([]mappingItem, error) {
128+
var mapping []mappingItem
129+
projectIDStr := envs.Get(ProjectIDOverrideENV)
130+
branchIDStr := envs.Get(BranchIDOverrideENV)
131+
132+
if branchIDStr != "" && len(content.Branches) != 1 {
133+
return nil, errors.Errorf(`env %s=%s can be used if there is one branch in the manifest, found %d branches`, BranchIDOverrideENV, branchIDStr, len(content.Branches))
134+
}
135+
136+
if projectIDStr != "" {
137+
projectIDInt, err := strconv.Atoi(projectIDStr)
138+
if err != nil {
139+
return nil, errors.Errorf(`env %s=%s is not valid project ID`, ProjectIDOverrideENV, projectIDStr)
140+
}
141+
projectID := keboola.ProjectID(projectIDInt)
142+
if projectID != content.Project.ID {
143+
logger.Infof(ctx, `Overriding the project ID by the environment variable %s=%v`, ProjectIDOverrideENV, projectID)
144+
mapping = append(mapping, mappingItem{
145+
ManifestValue: content.Project.ID,
146+
MemoryValue: projectID,
147+
})
148+
}
149+
}
150+
151+
if branchIDStr != "" {
152+
branchIDInt, err := strconv.Atoi(branchIDStr)
153+
if err != nil {
154+
return nil, errors.Errorf(`env %s=%s is not valid branch ID`, BranchIDOverrideENV, branchIDStr)
155+
}
156+
originalBranchID := content.Branches[0].ID
157+
replacedBranchID := keboola.BranchID(branchIDInt)
158+
if replacedBranchID != content.Branches[0].ID {
159+
logger.Infof(ctx, `Overriding the branch ID by the environment variable %s=%v`, BranchIDOverrideENV, replacedBranchID)
160+
// Map branch ID in all objects
161+
mapping = append(mapping, mappingItem{
162+
ManifestValue: originalBranchID,
163+
MemoryValue: replacedBranchID,
164+
})
165+
// Map allowed branches filter
166+
mapping = append(mapping, mappingItem{
167+
ManifestValue: model.AllowedBranch(originalBranchID.String()),
168+
MemoryValue: model.AllowedBranch(replacedBranchID.String()),
169+
})
170+
}
171+
// Replace main branch in the filter, with branch ID, if needed
172+
if len(content.AllowedBranches) == 1 && content.AllowedBranches[0] == model.MainBranchDef {
173+
content.AllowedBranches[0] = model.AllowedBranch(replacedBranchID.String())
174+
}
175+
}
176+
177+
return mapping, nil
178+
}
179+
170180
func (m *Manifest) Save(ctx context.Context, fs filesystem.Fs) error {
171181
// Create file content
172182
content := newFile(m.ProjectID(), m.APIHost())

‎internal/pkg/service/common/configmap/bind.go‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -369,18 +369,24 @@ func collectValues(cfg BindConfig, flagToField FlagToFieldFn) (*orderedmap.Order
369369

370370
// Bind flags and ENVs, flags have priority
371371
cfg.Flags.VisitAll(func(flag *pflag.Flag) {
372-
if fieldPath, ok := flagToField(flag); ok {
373-
if flag.Changed {
374-
if err := values.SetNestedPath(fieldPath, fieldValue{Value: flag.Value.String(), SetBy: SetByFlag}); err != nil {
372+
fieldPath, ok := flagToField(flag)
373+
if !ok {
374+
return
375+
}
376+
377+
if flag.Changed {
378+
if err := values.SetNestedPath(fieldPath, fieldValue{Value: flag.Value.String(), SetBy: SetByFlag}); err != nil {
379+
errs.Append(err)
380+
}
381+
return
382+
}
383+
384+
if cfg.EnvNaming != nil && cfg.Envs != nil {
385+
envName := cfg.EnvNaming.FlagToEnv(flag.Name)
386+
if envValue, found := cfg.Envs.Lookup(envName); found {
387+
if err := values.SetNestedPath(fieldPath, fieldValue{Value: envValue, SetBy: SetByEnv}); err != nil {
375388
errs.Append(err)
376389
}
377-
} else if cfg.EnvNaming != nil && cfg.Envs != nil {
378-
envName := cfg.EnvNaming.FlagToEnv(flag.Name)
379-
if envValue, found := cfg.Envs.Lookup(envName); found {
380-
if err := values.SetNestedPath(fieldPath, fieldValue{Value: envValue, SetBy: SetByEnv}); err != nil {
381-
errs.Append(err)
382-
}
383-
}
384390
}
385391
}
386392
})

0 commit comments

Comments
 (0)