gin icon indicating copy to clipboard operation
gin copied to clipboard

Suggestion: Move Print Debug to Run() functions?

Open 0xArch3r opened this issue 2 months ago • 7 comments

Description

Currently, the debugPrintWARNINGNew() function runs when you call gin.New(). This can, under the right circumstances, cause confusions to the developer if they call gin.New() and then call gin.SetMode(gin.ReleaseMode) after that .New().

The debug warning will print, even though the developer did set the mode to release mode before actually running the gin server.

Would it not make more sense to check and print the debug message when we actually go to run the server?

How to reproduce

package main

import (
        "fmt"
	"github.com/gin-gonic/gin"
)

func main() {
	g := gin.New()
	gin.SetMode(gin.ReleaseMode)
	g.Run(":9000")
        fmt.Println("running")
}

Expectations

$ go run main.go
running

Actual result

$ go run main.go
[GIN-debug] [WARNING] Running in "debug" mode. Switch to "release" mode in production.
 - using env:   export GIN_MODE=release
 - using code:  gin.SetMode(gin.ReleaseMode)
running

Environment

  • go version: doesn't matter
  • gin version (or commit ref): doesn't matter
  • operating system: doesn't matter

0xArch3r avatar Apr 11 '24 17:04 0xArch3r

package main

import (
        "fmt"
        "os"
        "github.com/gin-gonic/gin"
)

func main() {
        os.Setenv("GIN_MODE", "release")
        g := gin.New()
        gin.SetMode(gin.ReleaseMode)
        g.Run(":9000")
        fmt.Println("running")
}

if you insert os.Setenv("GIN_MODE", "release") one line or

Enter environment variables export GIN_MODE=release in command line before go run main.go

you get Expected result

Kim-Hyo-Bin avatar Apr 12 '24 08:04 Kim-Hyo-Bin

I think a new Gin instance has been New and should be set up around it like this:

package main

import (
	"fmt"

	"github.com/gin-gonic/gin"
)

func main() {
	g1 := gin.New()
	g1.SetMode(gin.ReleaseMode)
	g1.Run(":9001")

	g2 := gin.New()
	g2.SetMode(gin.DebugMode)
	g2.Run(":9002")
	fmt.Println("running")
}

FarmerChillax avatar Apr 12 '24 09:04 FarmerChillax

I think a new Gin instance has been New and should be set up around it like this:

package main

import (
	"fmt"

	"github.com/gin-gonic/gin"
)

func main() {
	g1 := gin.New()
	g1.SetMode(gin.ReleaseMode)
	g1.Run(":9001")

	g2 := gin.New()
	g2.SetMode(gin.DebugMode)
	g2.Run(":9002")
	fmt.Println("running")
}
  1. Gin Mode is a global variable in the package, it isn’t a per instance setting. The gin.Engine doesn’t have a method SetMode
  2. Let’s say mode was per instance, the way your code is, both instances would print the debug warning even though the first one you’re running in release mode @FarmerChillax

Unless you're saying, what you put with g2.SetMode is how you THINK it should be. If so, I could get behind that.

0xArch3r avatar Apr 12 '24 11:04 0xArch3r

I think a new Gin instance has been New and should be set up around it like this:

package main

import (
	"fmt"

	"github.com/gin-gonic/gin"
)

func main() {
	g1 := gin.New()
	g1.SetMode(gin.ReleaseMode)
	g1.Run(":9001")

	g2 := gin.New()
	g2.SetMode(gin.DebugMode)
	g2.Run(":9002")
	fmt.Println("running")
}
  1. Gin Mode is a global variable in the package, it isn’t a per instance setting. The gin.Engine doesn’t have a method SetMode
  2. Let’s say mode was per instance, the way your code is, both instances would print the debug warning even though the first one you’re running in release mode @FarmerChillax

Unless you're saying, what you put with g2.SetMode is how you THINK it should be. If so, I could get behind that.

The point I'm making is just that it's confusing to use the current API and it feels more natural to set up each instance.(for discussion purposes)

Regardless of current usage.(Maybe I should open a new issue)

FarmerChillax avatar Apr 15 '24 01:04 FarmerChillax

I think a new Gin instance has been New and should be set up around it like this:

package main

import (
	"fmt"

	"github.com/gin-gonic/gin"
)

func main() {
	g1 := gin.New()
	g1.SetMode(gin.ReleaseMode)
	g1.Run(":9001")

	g2 := gin.New()
	g2.SetMode(gin.DebugMode)
	g2.Run(":9002")
	fmt.Println("running")
}
  1. Gin Mode is a global variable in the package, it isn’t a per instance setting. The gin.Engine doesn’t have a method SetMode
  2. Let’s say mode was per instance, the way your code is, both instances would print the debug warning even though the first one you’re running in release mode @FarmerChillax

Unless you're saying, what you put with g2.SetMode is how you THINK it should be. If so, I could get behind that.

The point I'm making is just that it's confusing to use the current API and it feels more natural to set up each instance.(for discussion purposes)

Regardless of current usage.(Maybe I should open a new issue)

Alright, yeah then you're further proving my point that it isnt so straight forward. Cause even in your suggested design, you would still get the Debug mention when you do g1 := gin.New() because the Gin MOde is declared after, which isnt what one would expect (At least isnt waht I would expect)

I think that debug warning shouldnt be called until the gin server is actually called upon to ListenAndServe. I can also see a value in it being on a per instance basis like you're suggesting as well.

0xArch3r avatar Apr 15 '24 17:04 0xArch3r

I think a new Gin instance has been New and should be set up around it like this:

package main

import (
	"fmt"

	"github.com/gin-gonic/gin"
)

func main() {
	g1 := gin.New()
	g1.SetMode(gin.ReleaseMode)
	g1.Run(":9001")

	g2 := gin.New()
	g2.SetMode(gin.DebugMode)
	g2.Run(":9002")
	fmt.Println("running")
}
  1. Gin Mode is a global variable in the package, it isn’t a per instance setting. The gin.Engine doesn’t have a method SetMode
  2. Let’s say mode was per instance, the way your code is, both instances would print the debug warning even though the first one you’re running in release mode @FarmerChillax

Unless you're saying, what you put with g2.SetMode is how you THINK it should be. If so, I could get behind that.

The point I'm making is just that it's confusing to use the current API and it feels more natural to set up each instance.(for discussion purposes) Regardless of current usage.(Maybe I should open a new issue)

Alright, yeah then you're further proving my point that it isnt so straight forward. Cause even in your suggested design, you would still get the Debug mention when you do g1 := gin.New() because the Gin MOde is declared after, which isnt what one would expect (At least isnt waht I would expect)

I think that debug warning shouldnt be called until the gin server is actually called upon to ListenAndServe. I can also see a value in it being on a per instance basis like you're suggesting as well.

Yes, I agree with your point too. I hope that while improving the API, I will also correct the debug information you mentioned.

FarmerChillax avatar Apr 16 '24 01:04 FarmerChillax

There is a problem with the execution location of gin.SetMode

package main

import (
	"net/http"

	"github.com/gin-gonic/gin"
)

func main() {
	gin.SetMode(gin.ReleaseMode)
	g := gin.New()
	g.GET("/", func(c *gin.Context) {
		c.JSON(http.StatusOK,"ok")
	})
	g.Run(":9000")
}

I have tried a variety of assignment methods, but the effect is not ideal. I may only be able to achieve consistent and good results under gin.Engine.

package main

import (
	"fmt"
	"net/http"
	"sync"

	"github.com/gin-gonic/gin"
)
var w sync.WaitGroup

func main() {
	for _,p := range []struct {
			port string
			mod  string
	}{
		{
			port: ":9000",
			mod: gin.ReleaseMode,
		},
		{
			port: ":9001",
			mod: gin.DebugMode,
		},
	}{
		w.Add(1)
		go func(port,mod string) {
			gin.Engine{}
			gin.SetMode(mod)
			fmt.Println(gin.Mode())
			r := gin.New()
			r.GET("/"+port, func(c *gin.Context) {
				c.JSON(http.StatusOK,"ok")
			})
			r.Run(port)
		}(p.port,p.mod)
	}	
	w.Wait()
}

RedCrazyGhost avatar Apr 19 '24 07:04 RedCrazyGhost