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

Fail when cluster commands are called without a proper workspace set #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fail when cluster commands are called without a proper workspace set #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 7 additions & 1 deletion cmd/migrate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ var migrateCmd = &cobra.Command{
ctx := context.Background()
orbs := lo.Map(cluster.Config().Orbs, func(cfg orb.OrbCfg, _ int) string { return cfg.Name })
err = migrate(ctx, cluster, dbReset, orbs, orbs)

if err != nil {
log.Fatal(err)
}
},
}

Expand All @@ -41,6 +43,10 @@ func migrate(ctx context.Context, cluster orb.OrbCluster, dbReset bool, orbs []s
err = errors.New("orbs and databases have to be of the same size")
return
}
if len(orbs) == 0 {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this change is a stowaway here, just was testing and thought it would be nice to cover this case even if remote.

err = errors.New("You need at least one configured Orb to migrate")
return
}
logger := log.New(os.Stdout)
logger.SetReportTimestamp(true)
logger.Info("Starting migration...")
Expand Down
9 changes: 2 additions & 7 deletions orb/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ package orb

import (
"errors"
"fmt"
"github.com/omnigres/cli/internal/fileutils"
"github.com/spf13/viper"
"os"
"path/filepath"
)

Expand Down Expand Up @@ -62,12 +62,7 @@ func LoadConfig(path string) (cfg *Config, err error) {
v.SetConfigFile(filepath.Join(path, "omnigres.yaml"))
err = v.ReadInConfig()
if err != nil {
if _, ok := err.(*os.PathError); ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this change may affect omnigres run as it can run files from a directory without a yaml file – so handling the error there might be needed.

This assumes we agree that omnigres run should be able to run yaml-less directories (by just throwing in a default config as the removed code did) – so we'll need to handle it there, in run.go – if the file is not found (error is of a certain type denoting that) then supply NewConfig()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was actually dropping that behavior (running on yaml-less dirs) for the sake of simplicity. I thought run would be analogous to start in that sense.
Should we keep this behavior for the run command?

cfg = NewConfig()
err = nil
return
}
return
return nil, fmt.Errorf("omnigres.yaml not found in %s. Try setting a different workspace with -w or running omnigres init to create a new project config.", path)
}

cfg = NewConfig()
Expand Down