Skip to content

Commit defbe8f

Browse files
mwbrookszimeg
andauthored
fix(python): 'create' and 'init' use warnings instead of errors for non-fatal pyproject.toml issues (#406)
* fix: use warnings instead of errors for non-fatal pyproject.toml issues When installPyProjectToml encounters missing structure (no [project] section, no dependencies array, invalid TOML, or file not found), these are not true errors — they just mean the pyproject.toml doesn't have the structure needed to auto-add slack-cli-hooks. The CLI now logs a debug warning and returns a descriptive "Skipped updating pyproject.toml because <reason>" message instead of propagating an error. True errors (file write failures, pip install failures) remain errors. * test: remove duplicate tests * Update internal/runtime/python/python.go Co-authored-by: Eden Zimbelman <eden.zimbelman@salesforce.com> --------- Co-authored-by: Eden Zimbelman <eden.zimbelman@salesforce.com>
1 parent 4998899 commit defbe8f

File tree

2 files changed

+39
-25
lines changed

2 files changed

+39
-25
lines changed

internal/runtime/python/python.go

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -216,12 +216,13 @@ func installRequirementsTxt(fs afero.Fs, projectDirPath string) (output string,
216216
}
217217

218218
// installPyProjectToml handles adding slack-cli-hooks to pyproject.toml
219-
func installPyProjectToml(fs afero.Fs, projectDirPath string) (output string, err error) {
219+
func installPyProjectToml(ctx context.Context, ios iostreams.IOStreamer, fs afero.Fs, projectDirPath string) (output string, err error) {
220220
pyProjectFilePath := filepath.Join(projectDirPath, "pyproject.toml")
221221

222222
file, err := afero.ReadFile(fs, pyProjectFilePath)
223223
if err != nil {
224-
return fmt.Sprintf("Error reading pyproject.toml: %s", err), err
224+
ios.PrintDebug(ctx, "Warning: could not read pyproject.toml: %s", err)
225+
return "Skipped updating pyproject.toml because file cannot be read", nil
225226
}
226227

227228
fileData := string(file)
@@ -235,25 +236,26 @@ func installPyProjectToml(fs afero.Fs, projectDirPath string) (output string, er
235236
var config map[string]interface{}
236237
err = toml.Unmarshal(file, &config)
237238
if err != nil {
238-
return fmt.Sprintf("Error parsing pyproject.toml: %s", err), err
239+
ios.PrintDebug(ctx, "Warning: could not parse pyproject.toml: %s", err)
240+
return "Skipped updating pyproject.toml because invalid TOML", nil
239241
}
240242

241243
// Verify `project` section and `project.dependencies` array exist
242244
projectSection, exists := config["project"]
243245
if !exists {
244-
err := fmt.Errorf("pyproject.toml missing project section")
245-
return fmt.Sprintf("Error updating pyproject.toml: %s", err), err
246+
ios.PrintDebug(ctx, "Warning: pyproject.toml missing [project] section")
247+
return "Skipped updating pyproject.toml because project section missing", nil
246248
}
247249

248250
projectMap, ok := projectSection.(map[string]interface{})
249251
if !ok {
250-
err := fmt.Errorf("pyproject.toml project section is not a valid format")
251-
return fmt.Sprintf("Error updating pyproject.toml: %s", err), err
252+
ios.PrintDebug(ctx, "Warning: pyproject.toml [project] section is not a valid format")
253+
return "Skipped updating pyproject.toml because project section invalid", nil
252254
}
253255

254256
if _, exists := projectMap["dependencies"]; !exists {
255-
err := fmt.Errorf("pyproject.toml missing dependencies array")
256-
return fmt.Sprintf("Error updating pyproject.toml: %s", err), err
257+
ios.PrintDebug(ctx, "Warning: pyproject.toml missing dependencies array in [project] section")
258+
return "Skipped updating pyproject.toml because dependencies missing", nil
257259
}
258260

259261
// Use string manipulation to add the dependency while preserving formatting.
@@ -264,8 +266,8 @@ func installPyProjectToml(fs afero.Fs, projectDirPath string) (output string, er
264266
matches := dependenciesRegex.FindStringSubmatch(fileData)
265267

266268
if len(matches) == 0 {
267-
err := fmt.Errorf("pyproject.toml missing dependencies array")
268-
return fmt.Sprintf("Error updating pyproject.toml: %s", err), err
269+
ios.PrintDebug(ctx, "Warning: pyproject.toml dependencies array could not be located")
270+
return "Skipped updating pyproject.toml because dependencies missing", nil
269271
}
270272

271273
prefix := matches[1] // "...dependencies = ["
@@ -347,16 +349,20 @@ func (p *Python) InstallProjectDependencies(ctx context.Context, projectDirPath
347349
// Handle requirements.txt if it exists
348350
if hasRequirementsTxt {
349351
output, err := installRequirementsTxt(fs, projectDirPath)
350-
outputs = append(outputs, output)
352+
if output != "" {
353+
outputs = append(outputs, output)
354+
}
351355
if err != nil {
352356
errs = append(errs, err)
353357
}
354358
}
355359

356360
// Handle pyproject.toml if it exists
357361
if hasPyProjectToml {
358-
output, err := installPyProjectToml(fs, projectDirPath)
359-
outputs = append(outputs, output)
362+
output, err := installPyProjectToml(ctx, ios, fs, projectDirPath)
363+
if output != "" {
364+
outputs = append(outputs, output)
365+
}
360366
if err != nil {
361367
errs = append(errs, err)
362368
}

internal/runtime/python/python_test.go

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,13 @@ dependencies = ["pytest==8.3.2"]`,
229229
}
230230
for name, tc := range tests {
231231
t.Run(name, func(t *testing.T) {
232+
ctx := slackcontext.MockContext(t.Context())
232233
fs := slackdeps.NewFsMock()
234+
os := slackdeps.NewOsMock()
235+
os.AddDefaultMocks()
236+
cfg := config.NewConfig(fs, os)
237+
ios := iostreams.NewIOStreamsMock(cfg, fs, os)
238+
ios.AddDefaultMocks()
233239
projectDirPath := "/path/to/project"
234240

235241
// Create pyproject.toml
@@ -240,7 +246,7 @@ dependencies = ["pytest==8.3.2"]`,
240246
require.NoError(t, err)
241247

242248
// Test
243-
output, err := installPyProjectToml(fs, projectDirPath)
249+
output, err := installPyProjectToml(ctx, ios, fs, projectDirPath)
244250

245251
// Assertions
246252
if tc.expectedError {
@@ -249,7 +255,9 @@ dependencies = ["pytest==8.3.2"]`,
249255
require.NoError(t, err)
250256
}
251257

252-
require.Contains(t, output, tc.expectedOutput)
258+
if tc.expectedOutput != "" {
259+
require.Contains(t, output, tc.expectedOutput)
260+
}
253261

254262
// Verify file content contains expected strings
255263
content, err := afero.ReadFile(fs, pyprojectPath)
@@ -527,29 +535,29 @@ dependencies = [
527535
expectedOutputs: []string{"Updated requirements.txt"},
528536
expectedError: false,
529537
},
530-
"Error when pyproject.toml has no dependencies array": {
538+
"Warning when pyproject.toml has no dependencies array": {
531539
existingFiles: map[string]string{
532540
"pyproject.toml": `[project]
533541
name = "my-app"`,
534542
},
535-
expectedOutputs: []string{"Error updating pyproject.toml: pyproject.toml missing dependencies array"},
536-
expectedError: true,
543+
expectedOutputs: []string{"Skipped updating pyproject.toml because dependencies missing"},
544+
expectedError: false,
537545
},
538-
"Error when pyproject.toml has no [project] section": {
546+
"Warning when pyproject.toml has no [project] section": {
539547
existingFiles: map[string]string{
540548
"pyproject.toml": `[tool.black]
541549
line-length = 88`,
542550
},
543-
expectedOutputs: []string{"Error updating pyproject.toml: pyproject.toml missing project section"},
544-
expectedError: true,
551+
expectedOutputs: []string{"Skipped updating pyproject.toml because project section missing"},
552+
expectedError: false,
545553
},
546-
"Error when pyproject.toml is invalid TOML": {
554+
"Warning when pyproject.toml is invalid TOML": {
547555
existingFiles: map[string]string{
548556
"pyproject.toml": `[project
549557
name = "broken`,
550558
},
551-
expectedOutputs: []string{"Error parsing pyproject.toml"},
552-
expectedError: true,
559+
expectedOutputs: []string{"Skipped updating pyproject.toml because invalid TOML"},
560+
expectedError: false,
553561
},
554562
}
555563
for name, tc := range tests {

0 commit comments

Comments
 (0)