Skip to content
This repository has been archived by the owner on May 30, 2022. It is now read-only.

Disable gin debug logging on demand #299

Conversation

dottorblaster
Copy link
Contributor

@dottorblaster dottorblaster commented Oct 1, 2021

Since Gin's debug logs in tests are super noisy to us,I thought that it would be a nice idea to change the behavior using a TRENTO_DISABLE_GIN_DEBUG_LOG env var :-)

Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

@dottorblaster I'm not sure if we need this.
gin-gonic already has an internal Env variable for this GIN_MODE.
Maybe, we should just document how to use it.
And we could set this variable to test in our tests

Ref:
https://github.com/gin-gonic/gin/blob/master/mode.go#L15

@dottorblaster
Copy link
Contributor Author

@arbulu89 wow, I think you're right 😮

@arbulu89
Copy link
Contributor

arbulu89 commented Oct 4, 2021

@arbulu89 wow, I think you're right open_mouth

I actually have been testing, and I didn't make it work... I don't even know if this init method to set the mode is actually called anywhere.

I just did the next:

Add the next snippet in the NewAppWithDeps"

mode := os.Getenv(gin.EnvGinMode)
gin.SetMode(mode)

In the Makefile, change the test call to GIN_MODE=test go test -v ./...

This makes the test output cleaner

@dottorblaster
Copy link
Contributor Author

Ah yeah absolutely, it worked for me because I was doing this for testing purposes and with make test it worked seamlessly :)

@stefanotorresi
Copy link
Member

So, what are we going to do? I would be totally fine with what Xabi just suggested, and defer the integration with the --log-level flag to another time.

@dottorblaster
Copy link
Contributor Author

@stefanotorresi @arbulu89 I like that, I'll update the PR accordingly, thank you so much!

@dottorblaster dottorblaster force-pushed the make_gin_debug_log_configurable branch from 518310e to bfba591 Compare October 4, 2021 15:22
stefanotorresi
stefanotorresi previously approved these changes Oct 4, 2021
Copy link
Member

@stefanotorresi stefanotorresi left a comment

Choose a reason for hiding this comment

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

approved with nitpick 😜

web/app.go Outdated Show resolved Hide resolved
Copy link
Member

@stefanotorresi stefanotorresi left a comment

Choose a reason for hiding this comment

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

you rule! 🍻

@stefanotorresi stefanotorresi merged commit 1d0575b into trento-project:main Oct 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants