Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor error handling #303

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Refactor error handling #303

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0f9b045
commands: Rework newQemuHelper to bubble-up error instead of panicing
obbardc Jan 7, 2022
a3fbde3
Rework error handling to check context.State
obbardc Jul 26, 2023
64d043d
Fix typo in stage name Postmachine→PostMachine
obbardc Jan 5, 2022
aa1e5a0
debos: Improve error messages
obbardc Mar 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 20 additions & 21 deletions cmd/debos/debos.go
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mention "this paradigm is quite fragile" but all this change seems to do is rather then having a local variable track the error state (in the form of error code) you moved it to marking it in the context. I'm not really sure why that's less fragile ;)

it does make the code a big more clear though so seems reasonable

Original file line number Diff line number Diff line change
Expand Up @@ -50,37 +50,36 @@ func warnLocalhost(variable string, value string) {
Consider using an address that is valid on your network.`

if strings.Contains(value, "localhost") ||
strings.Contains(value, "127.0.0.1") ||
strings.Contains(value, "::1") {
strings.Contains(value, "127.0.0.1") ||
strings.Contains(value, "::1") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these don't look like typo fixes

log.Printf(message, variable)
}
}


func main() {
context := debos.DebosContext { &debos.CommonContext{}, "", "" }
context := debos.DebosContext{&debos.CommonContext{}, "", ""}
var options struct {
Backend string `short:"b" long:"fakemachine-backend" description:"Fakemachine backend to use" default:"auto"`
ArtifactDir string `long:"artifactdir" description:"Directory for packed archives and ostree repositories (default: current directory)"`
InternalImage string `long:"internal-image" hidden:"true"`
TemplateVars map[string]string `short:"t" long:"template-var" description:"Template variables (use -t VARIABLE:VALUE syntax)"`
DebugShell bool `long:"debug-shell" description:"Fall into interactive shell on error"`
Shell string `short:"s" long:"shell" description:"Redefine interactive shell binary (default: bash)" optionsl:"" default:"/bin/bash"`
ScratchSize string `long:"scratchsize" description:"Size of disk backed scratch space"`
CPUs int `short:"c" long:"cpus" description:"Number of CPUs to use for build VM (default: 2)"`
Memory string `short:"m" long:"memory" description:"Amount of memory for build VM (default: 2048MB)"`
ShowBoot bool `long:"show-boot" description:"Show boot/console messages from the fake machine"`
EnvironVars map[string]string `short:"e" long:"environ-var" description:"Environment variables (use -e VARIABLE:VALUE syntax)"`
Verbose bool `short:"v" long:"verbose" description:"Verbose output"`
PrintRecipe bool `long:"print-recipe" description:"Print final recipe"`
DryRun bool `long:"dry-run" description:"Compose final recipe to build but without any real work started"`
DisableFakeMachine bool `long:"disable-fakemachine" description:"Do not use fakemachine."`
Backend string `short:"b" long:"fakemachine-backend" description:"Fakemachine backend to use" default:"auto"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more spurious changes that aren't typo fixes

ArtifactDir string `long:"artifactdir" description:"Directory for packed archives and ostree repositories (default: current directory)"`
InternalImage string `long:"internal-image" hidden:"true"`
TemplateVars map[string]string `short:"t" long:"template-var" description:"Template variables (use -t VARIABLE:VALUE syntax)"`
DebugShell bool `long:"debug-shell" description:"Fall into interactive shell on error"`
Shell string `short:"s" long:"shell" description:"Redefine interactive shell binary (default: bash)" optionsl:"" default:"/bin/bash"`
ScratchSize string `long:"scratchsize" description:"Size of disk backed scratch space"`
CPUs int `short:"c" long:"cpus" description:"Number of CPUs to use for build VM (default: 2)"`
Memory string `short:"m" long:"memory" description:"Amount of memory for build VM (default: 2048MB)"`
ShowBoot bool `long:"show-boot" description:"Show boot/console messages from the fake machine"`
EnvironVars map[string]string `short:"e" long:"environ-var" description:"Environment variables (use -e VARIABLE:VALUE syntax)"`
Verbose bool `short:"v" long:"verbose" description:"Verbose output"`
PrintRecipe bool `long:"print-recipe" description:"Print final recipe"`
DryRun bool `long:"dry-run" description:"Compose final recipe to build but without any real work started"`
DisableFakeMachine bool `long:"disable-fakemachine" description:"Do not use fakemachine."`
}

// These are the environment variables that will be detected on the
// host and propagated to fakemachine. These are listed lower case, but
// they are detected and configured in both lower case and upper case.
var environ_vars = [...]string {
var environ_vars = [...]string{
"http_proxy",
"https_proxy",
"ftp_proxy",
Expand Down Expand Up @@ -331,7 +330,7 @@ func main() {

for _, a := range r.Actions {
err = a.PostMachine(&context)
if handleError(&context, err, a, "Postmachine") {
if handleError(&context, err, a, "PostMachine") {
return
}
}
Expand Down