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.

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

Introduce Logger #369

Merged
merged 3 commits into from Jun 21, 2016

Conversation

@nicolai86
Copy link
Contributor

commented Jun 20, 2016

Following #368 this PR introduces a Logger interface which allows us to extract the http2curl and logrus dependencies into the cli & command packages.

The Logger contains two functions - LogHTTP to handle http.Request, and Log for plain string.
The cli configures the ScalewayAPI instance to retain the old logging behaviour. By default, the logging is not very verbose, logging only messages & API endpoints being used.

Also, I've replaced anonuuid with static, fake uuids. Not sure why we need "randomized" uuids if the only concern is to sanitize logs from real credentials.

Log(...interface{})
}

func NewDefaultLogger() Logger {

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jun 20, 2016

exported function NewDefaultLogger should have comment or be unexported

"os"
)

type Logger interface {

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jun 20, 2016

exported type Logger should have comment or be unexported

@@ -0,0 +1,33 @@
// Copyright (C) 2015 Scaleway. All rights reserved.

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jun 20, 2016

package comment should be of the form "Package api ..."

// error.Fields handling
for k, v := range e.Fields {
log.Debugf(" %-30s %s", fmt.Sprintf("%s: ", k), v)
func (s *ScalewayAPI) HideAPICredentials(input string) string {

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jun 20, 2016

exported method ScalewayAPI.HideAPICredentials should have comment or be unexported

nicolai86 and others added 3 commits Jun 20, 2016
@nicolai86

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2016

@QuentinPerez thanks for improving the PR! If you have other changes in mind let me know; also if you want to have the commits squashed or rebased, to conform commit msg guidelines.

@QuentinPerez

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2016

LGTM, ping @moul 😊

@moul

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2016

LGTM, thank you

@moul moul merged commit a8dd2cf into scaleway:master Jun 21, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
hound No violations found. Woof!
@moul

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2016

Well done guys

@moul

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2016

@QuentinPerez can you credit @nicolai86 in the changelog ?

@nicolai86

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2016

Thanks for being so responsive! I'll continue my quest to get scaleway into terraform, then. 🙏🏼

@nicolai86

This comment has been minimized.

Copy link
Contributor Author

commented Jun 25, 2016

For those interested in the progress of the terraform provider, you can take a look here: hashicorp/terraform#7331

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.