Skip to content

Commit

Permalink
Merge branch 'slice-flags' into 'develop'
Browse files Browse the repository at this point in the history
Implement generated slice flags for VirtualMachineName, GroupName and AccountName

See merge request open-source/bytemark-client!302
  • Loading branch information
Hannah Pirie committed Dec 13, 2018
2 parents 20a728f + bd8a65f commit 4e39a35
Show file tree
Hide file tree
Showing 20 changed files with 681 additions and 39 deletions.
1 change: 1 addition & 0 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ quality-test:
script:
- cd $GOPATH/src/github.com/BytemarkHosting/bytemark-client
- export GOPATH
- go get golang.org/x/tools/cmd/goimports
- go install ./...
- go generate -tags quality $(go list ./... | grep -v vendor)
- go test -run 'TestQuality.*' -tags quality ./...
Expand Down
2 changes: 2 additions & 0 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 20 additions & 8 deletions HACKING
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
How to hack on bytemark-client
==============================

bytemark-client is a big go project with a lot of code (> 20,000 SLOC at last
bytemark-client is a big go project with a lot of code (> 30,000 SLOC at last
count) and a lot of packages. It's divided up into two main big chunks -
the command-line client in cmd/bytemark, and the API client library in lib.

Expand All @@ -24,7 +24,7 @@ client:
compile slices of cli.Commands in the commands/* packages and cmd/bytemark.
2. cmd/bytemark.main() is run, which runs some other functions in
cmd/bytemark to set up a cmd/bytemark/config.Manager and flag.FlagSet
3. cmd/bytemark.main combines the commands slices together, with
3. cmd/bytemark.Commands() combines the commands slices together, with
cmd/bytemark/commands/admin if the --admin flag is set, then creates a
urfave/cli.App using cmd/bytemark/app.BaseAppSetup
4. cmd/bytemark.main creates a new lib.Client, then attaches that and the
Expand Down Expand Up @@ -53,7 +53,7 @@ pretty normal command, in terms of its implementation. It's defined in show.go
Here's the source for its Action. (app here being cmd/bytemark/app)

Action: app.Action(args.Optional("account"), with.Account("account"), func(c *app.Context) error {
return c.OutputInDesiredForm(c.Account)
return c.OutputInDesiredForm(c.Account)
}),

app.Action returns a `func(c *cli.Context) error`, and takes
Expand Down Expand Up @@ -90,8 +90,18 @@ Here's a tree of the folders in this repo with descriptions of what each is for
│   │   ├── auth - Authentication routine for the client, called via
│   │ │ with.EnsureAuth
│   │   ├── flags - new flag types that are used in the client (e.g.
│   │ │ VirtualMachineName, SizeSpec, ResizeSpec, AccountName,
│   │ │ Privilege)
│   │ │ │ VirtualMachineName, SizeSpec, ResizeSpec, AccountName,
│   │ │ │ Privilege)
│   │   │   └── gen - packages to generate flag code
│   │   │   └── slice_flags - generator for SliceFlags - currently
│   │   │ AccountName, GroupName and VirtualMachineName.
│   │   │ More can be added by altering
│   │   │ app/flags/slice_flags.go and you may need to add
│   │   │ more preparation code to
│   │   │ gen/slice_flags/template_test.go.tmpl
│   │   │ Whenever you alter anything in slice_flags, run
│   │   │ `go generate ./...` from the bytemark-client dir
│   │   │ to make sure the slice flags are kept up-to-date.
│   │   ├── flagsets - common sets of flags & code to support reading complex
│   │ │ objects from them. e.g. VirtualMachineSpec & ImageInstall
│   │ ├── wait - functions which wait for a condition to be true before
Expand Down Expand Up @@ -125,8 +135,10 @@ Here's a tree of the folders in this repo with descriptions of what each is for
│   └── sizespec - Parser for size specifications (e.g 35GiB, +3MiB)
├── doc - folder of documentation stuff.
│ Also includes the manpage source (bytemark.asciidoc)
├── gen - scripts to aid the building/distributing process.
│ changelog.
├── gen - scripts to aid the building/distributing process. In particular, has
│ │ a changelog.sh script to generate a new changelog entry with all the
│ │ changes since the last commit to master.
│ └─ list_types - package to generate code for a list of types using sprintf.
├── lib - the main API client object, which makes requests.
│ │ At the moment, loads of requests are defined as methods on lib.Client,
│ │ but we want to move away from that and towards requests being
Expand Down Expand Up @@ -228,4 +240,4 @@ To generate the hash, use the following command.

find . -type d \! -path './.*' \! -path './vendor/*' | sort | shasum -a 256

# sha256sum: 2a2e8078c018b791fb8f4749ae2a3afec81a6eacec6caf36cfaa094ede7d82c5
# sha256sum: 993688b7f02276dcbd79d543b41f9994be2776ca163c8c40b6cc346435f834ce
8 changes: 4 additions & 4 deletions cmd/bytemark/app/flags/accessors.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ func GroupName(c *app.Context, flagname string) (gp lib.GroupName) {
if !ok {
return lib.GroupName{}
}
if gpNameFlag.GroupName == nil {
if gpNameFlag == nil {
return lib.GroupName{}
}
return *gpNameFlag.GroupName
return gpNameFlag.GroupName
}

// IPs returns the ips passed along as the named flag.
Expand Down Expand Up @@ -100,9 +100,9 @@ func VirtualMachineName(c *app.Context, flagname string) (vm lib.VirtualMachineN
if !ok {
return c.Config().GetVirtualMachine()
}
if vmNameFlag.VirtualMachineName == nil {
if vmNameFlag == nil {
return lib.VirtualMachineName{}
}

return *vmNameFlag.VirtualMachineName
return vmNameFlag.VirtualMachineName
}
21 changes: 15 additions & 6 deletions cmd/bytemark/app/flags/account_name.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,30 @@ import (

// AccountNameFlag is used for all --account flags, excluding the global one.
type AccountNameFlag struct {
AccountName string
Value string
// AccountName is the actual name that will be passed on to API calls, and
// is made by checking the contents of Value are a valid account. If Value
// is unset then the value of the 'account' config variable is used
AccountName string
// Value is the raw input to the flag, and can be used as the default when
// creating the flag.
Value string
// SetFromCommandLine is false by default but is set to true when Set is
// called. This allows setting a default by setting Value by yourself - Set
// is called from urfave/cli's flag-parsing code.
SetFromCommandLine bool
}

// Set runs lib.ParseAccountName to make sure we get just the 'pure' account name; no cluster / endpoint details
// Set sets Value and SetFromCommandLine on the flag
func (name *AccountNameFlag) Set(value string) error {
name.Value = value
name.SetFromCommandLine = true
return nil
}

// Preprocess sets the value of this flag to the global account flag if it's unset,
// and then runs lib.ParseAccountName. This is an implementation of `app.Preprocessor`,
// which is detected and called automatically by actions created with `app.Action`
// Preprocess sets the value of this flag to the global account flag if it's
// unset, and then runs lib.ParseAccountName to set AccountName. This is an
// implementation of `app.Preprocessor`, which is detected and called
// automatically by actions created with `app.Action`
func (name *AccountNameFlag) Preprocess(c *app.Context) (err error) {
if name.Value == "" {
name.Value = c.Context.GlobalString("account")
Expand Down
58 changes: 58 additions & 0 deletions cmd/bytemark/app/flags/account_name_slice.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package flags

// This file was automatically generate using
// cmd/bytemark/app/flags/gen/slice_flags - do not edit it by hand!
// To make your own SliceFlag, see the documentation in
// cmd/bytemark/app/flags/gen/slice_flags/slice_flags.go
// and edit cmd/bytemark/app/flags/slice_flags.go

import (
"strings"

"github.com/BytemarkHosting/bytemark-client/cmd/bytemark/app"
)

// AccountNameSliceFlag is used for AccountNameFlags that may be specified more than
// once. It's a slice of AccountNameFlag in order to avoid rewriting parsing
// logic.
type AccountNameSliceFlag []AccountNameFlag

// Preprocess calls Preprocess on all the underlying AccountNameFlags
func (sf *AccountNameSliceFlag) Preprocess(ctx *app.Context) error {
for i := range *sf {
err := (*sf)[i].Preprocess(ctx)
if err != nil {
return err
}
}
return nil
}

// Set appends a AccountNameFlag (created for you) to the slice
func (sf *AccountNameSliceFlag) Set(value string) error {
flag := AccountNameFlag{}
err := flag.Set(value)
if err != nil {
return err
}
*sf = append(*sf, flag)
return nil
}

// String returns all values in the slice, comma-delimeted
func (sf AccountNameSliceFlag) String() string {
strs := make([]string, len(sf))
for i, value := range sf {
strs[i] = value.String()
}
return strings.Join(strs, ", ")
}

// AccountNameSlice returns the named flag as a AccountNameSliceFlag,
// if it was one in the first place.
func AccountNameSlice(ctx *app.Context, name string) AccountNameSliceFlag {
if sf, ok := ctx.Context.Generic(name).(*AccountNameSliceFlag); ok {
return *sf
}
return AccountNameSliceFlag{}
}
53 changes: 53 additions & 0 deletions cmd/bytemark/app/flags/account_name_slice_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package flags_test

import (
"testing"

"github.com/BytemarkHosting/bytemark-client/cmd/bytemark/app"
"github.com/BytemarkHosting/bytemark-client/cmd/bytemark/app/flags"
"github.com/BytemarkHosting/bytemark-client/cmd/bytemark/testutil"
"github.com/BytemarkHosting/bytemark-client/lib"
"github.com/BytemarkHosting/bytemark-client/mocks"
"github.com/urfave/cli"
)

func TestAccountNameSliceFlag(t *testing.T) {
sf := flags.AccountNameSliceFlag{}
err := sf.Set("photocopier")
if err != nil {
t.Errorf("got error from Set(): %s", err)
}
if len(sf) != 1 {
t.Errorf("Expected len(AccountNameSliceFLag) to be 1, got %d",
len(sf))
}

t.Logf("Value: %s", sf[0].Value)
// it's a Preprocesser so we need to call Preprocess before we can validate
// String()
cfg, client, cliApp := testutil.BaseTestSetup(t, false, []cli.Command{})
cfg.When("GetIgnoreErr", "account").Return("default-account")
cfg.When("GetGroup").Return(lib.GroupName{Group: "default-group", Account: "default-account"})
cfg.When("GetVirtualMachine").Return(lib.VirtualMachineName{VirtualMachine: "default-server", Group: "default-group", Account: "default-account"})

// now some boilerplate to get a context
// TODO(telyn): this should probably be refactored out since it'll be
// wanted for basically every Preprocesser flag)
client.When("AuthWithToken", "test-token").Return(nil)
cliCtx := mocks.CliContext{}
cliCtx.When("App").Return(cliApp)
ctx := app.Context{
Context: &cliCtx,
}

// with a context we may now Preprocess
err = sf.Preprocess(&ctx)
if err != nil {
t.Errorf("Preprocess errored: %s", err)
}

if sf.String() != "photocopier" {
t.Errorf("Expected %q, got %q", "photocopier", sf.String())
}

}
Loading

0 comments on commit 4e39a35

Please sign in to comment.