Skip to content

Commit 5fb5d54

Browse files
committed
kepctl output: use logrus vs. Fprint Repo Out/Err
The Fprintf calls seem to be about avoiding logging to stdout so that tools like `jq` could consume structured output from `kepctl query`. Fortunately, logrus is already configured to do this, so use it instead. Changes were basically: - Where things wrote to r.Err, they are now logrus.Warns - Where things wrote to r.Out, they are now logrus.Infos - Drop the '\n' from the former Fprintf statements - Drop the StructuredFormats / suppressOuput logic `kepctl query --log-level debug --output json | jq .` is happy
1 parent 3ad8912 commit 5fb5d54

File tree

5 files changed

+18
-43
lines changed

5 files changed

+18
-43
lines changed

pkg/proposal/create.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"time"
2626

2727
"github.com/pkg/errors"
28+
"github.com/sirupsen/logrus"
2829

2930
"k8s.io/enhancements/api"
3031
"k8s.io/enhancements/pkg/repo"
@@ -74,7 +75,7 @@ func (c *CreateOpts) Validate(args []string) error {
7475
func Create(opts *CreateOpts) error {
7576
r := opts.Repo
7677

77-
fmt.Fprintf(r.Out, "Creating KEP %s %s %s\n", opts.SIG, opts.Number, opts.Name)
78+
logrus.Infof("Creating KEP %s %s %s", opts.SIG, opts.Number, opts.Name)
7879

7980
kep := &api.Proposal{}
8081

@@ -96,7 +97,7 @@ func Create(opts *CreateOpts) error {
9697
func createKEP(kep *api.Proposal, opts *CreateOpts) error {
9798
r := opts.Repo
9899

99-
fmt.Fprintf(r.Out, "Generating new KEP %s in %s ===>\n", opts.Name, opts.SIG)
100+
logrus.Infof("Generating new KEP %s in %s ===>", opts.Name, opts.SIG)
100101

101102
err := r.WriteKEP(kep)
102103
if err != nil {

pkg/proposal/promote.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package proposal
1919
import (
2020
"fmt"
2121

22+
"github.com/sirupsen/logrus"
2223
"k8s.io/enhancements/pkg/repo"
2324
)
2425

@@ -48,7 +49,7 @@ func (o *PromoteOpts) Validate(args []string) error {
4849
func Promote(opts *PromoteOpts) error {
4950
r := opts.Repo
5051

51-
fmt.Fprintf(r.Out, "Updating KEP %s/%s\n", opts.SIG, opts.Name)
52+
logrus.Infof("Updating KEP %s/%s", opts.SIG, opts.Name)
5253

5354
p, err := r.ReadKEP(opts.SIG, opts.Name)
5455
if err != nil {
@@ -65,7 +66,7 @@ func Promote(opts *PromoteOpts) error {
6566
}
6667

6768
// TODO: Implement ticketing workflow artifact generation
68-
fmt.Fprintf(r.Out, "KEP %s/%s updated\n", opts.SIG, opts.Name)
69+
logrus.Infof("KEP %s/%s updated", opts.SIG, opts.Name)
6970

7071
return nil
7172
}

pkg/repo/query.go

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,6 @@ var (
3737

3838
// DefaultOutputOpt is the default output format for kepctl query
3939
DefaultOutputOpt = "table"
40-
41-
StructuredOutputFormats = []string{
42-
"json",
43-
"yaml",
44-
"csv",
45-
}
4640
)
4741

4842
type QueryOpts struct {
@@ -104,19 +98,7 @@ func (r *Repo) PrepareQueryOpts(opts *QueryOpts) error {
10498
// Query searches the local repo and possibly GitHub for KEPs
10599
// that match the search criteria.
106100
func (r *Repo) Query(opts *QueryOpts) error {
107-
// if output format is json/yaml, suppress other outputs
108-
// json/yaml are structured formats, logging events which
109-
// do not conform to the spec will create formatting issues
110-
var suppressOutputs bool
111-
if sliceContains(StructuredOutputFormats, opts.Output) {
112-
suppressOutputs = true
113-
} else {
114-
suppressOutputs = false
115-
}
116-
117-
if !suppressOutputs {
118-
fmt.Fprintf(r.Out, "Searching for KEPs...\n")
119-
}
101+
logrus.Info("Searching for KEPs...")
120102

121103
err := r.PrepareQueryOpts(opts)
122104
if err != nil {
@@ -145,7 +127,7 @@ func (r *Repo) Query(opts *QueryOpts) error {
145127
if opts.IncludePRs {
146128
prKeps, err := r.loadKEPPullRequests(sig)
147129
if err != nil {
148-
fmt.Fprintf(r.Err, "error searching for KEP PRs from %s: %s\n", sig, err)
130+
logrus.Warnf("error searching for KEP PRs from %s: %s", sig, err)
149131
}
150132
if prKeps != nil {
151133
allKEPs = append(allKEPs, prKeps...)

pkg/repo/repo.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ func (r *Repo) loadKEPPullRequests(sig string) ([]*api.Proposal, error) {
414414
for k := range kepNames {
415415
kep, err := r.ReadKEP(sig, k)
416416
if err != nil {
417-
fmt.Fprintf(r.Err, "ERROR READING KEP %s: %s\n", k, err)
417+
logrus.Warnf("error reading KEP %v: %v", k, err)
418418
} else {
419419
kep.PRNumber = strconv.Itoa(pr.GetNumber())
420420
allKEPs = append(allKEPs, kep)

pkg/repo/write.go

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,13 @@ limitations under the License.
1717
package repo
1818

1919
import (
20+
"errors"
2021
"fmt"
2122
"io/ioutil"
2223
"os"
2324
"path/filepath"
2425

25-
"github.com/pkg/errors"
26+
"github.com/sirupsen/logrus"
2627
"gopkg.in/yaml.v3"
2728
"k8s.io/enhancements/api"
2829
)
@@ -44,25 +45,15 @@ func (r *Repo) WriteKEP(kep *api.Proposal) error {
4445
return errors.New("KEP name must be populated")
4546
}
4647

47-
if mkErr := os.MkdirAll(
48-
filepath.Join(
49-
r.ProposalPath,
50-
sig,
51-
kepName,
52-
),
53-
os.ModePerm,
54-
); mkErr != nil {
55-
return errors.Wrapf(mkErr, "creating KEP directory")
48+
kepPath := filepath.Join(r.ProposalPath, sig, kepName)
49+
logrus.Infof("creating KEP directory: %s", kepPath)
50+
if err = os.MkdirAll(kepPath, os.ModePerm); err != nil {
51+
return fmt.Errorf("unable to create KEP path %s: %w", kepPath, err)
5652
}
5753

58-
newPath := filepath.Join(
59-
r.ProposalPath,
60-
sig,
61-
kepName,
62-
ProposalMetadataFilename,
63-
)
54+
kepYamlPath := filepath.Join(kepPath, ProposalMetadataFilename)
6455

65-
fmt.Fprintf(r.Out, "writing KEP to %s\n", newPath)
56+
logrus.Infof("writing KEP metadata to %s", kepYamlPath)
6657

67-
return ioutil.WriteFile(newPath, b, os.ModePerm)
58+
return ioutil.WriteFile(kepYamlPath, b, os.ModePerm)
6859
}

0 commit comments

Comments
 (0)