ronantakizawa/codereview-bench huggingface dataset:
Repository: hasura/graphql-engine
File: cli/commands/migrate_create.go (Go)

Code before:
import (
	"io/ioutil"
	"os"
	"time"

	"github.com/ghodss/yaml"
	"github.com/hasura/graphql-engine/cli"
	"github.com/hasura/graphql-engine/cli/migrate"
	"github.com/pkg/errors"
	"github.com/spf13/cobra"
	"github.com/spf13/pflag"
	"github.com/spf13/viper"

	mig "github.com/hasura/graphql-engine/cli/migrate/cmd"
	log "github.com/sirupsen/logrus"
)

const migrateCreateCmdExamples = `
	# Setup migration files for the first time by introspecting a server:
  hasura migrate create "init" --from-server

	# Add admin secret for Hasura GraphQL Engine:
	hasura migrate create --admin-secret "<admin-secret>"

	# Add Hasura GraphQL Server endpoint:
	hasura migrate create --endpoint "<http-endpoint>"

	# Get SQL statements and Hasura metadata from the server:
	hasura migrate create "<migration_name>" --from-server
`

func newMigrateCreateCmd(ec *cli.ExecutionContext) *cobra.Command {
	v := viper.New()
	opts := &migrateCreateOptions{
		EC: ec,
	}

	migrateCreateCmd := &cobra.Command{
		Use:          "create [migration-name]",
		Short:        "Create files required for a migration",
		Long:         "Create sql and yaml files required for a migration",
		Example:      migrateCreateCmdExamples,
		SilenceUsage: true,
		Args:         cobra.ExactArgs(1),
		PreRunE: func(cmd *cobra.Command, args []string) error {
			ec.Viper = v
			return ec.Validate()
		},
		RunE: func(cmd *cobra.Command, args []string) error {
			opts.name = args[0]
			opts.EC.Spin("Creating migration files...")

Review comment (suggestion):
```suggestion
  hasura migrate create init --from-server --endpoint "<endpoint>"
```

Code after:
import (
	"io/ioutil"
	"os"
	"time"

	"github.com/ghodss/yaml"
	"github.com/hasura/graphql-engine/cli"
	"github.com/hasura/graphql-engine/cli/migrate"
	"github.com/pkg/errors"
	"github.com/spf13/cobra"
	"github.com/spf13/pflag"
	"github.com/spf13/viper"

	mig "github.com/hasura/graphql-engine/cli/migrate/cmd"
	log "github.com/sirupsen/logrus"
)

const migrateCreateCmdExamples = `  # Setup migration files for the first time by introspecting a server:
  hasura migrate create "init" --from-server

  # Use with admin secret:
  hasura migrate create --admin-secret "<admin-secret>"

  # Setup migration files from an instance mentioned by the flag:
  hasura migrate create init --from-server --endpoint "<endpoint>"`

func newMigrateCreateCmd(ec *cli.ExecutionContext) *cobra.Command {
	v := viper.New()
	opts := &migrateCreateOptions{
		EC: ec,
	}

	migrateCreateCmd := &cobra.Command{
		Use:          "create [migration-name]",
		Short:        "Create files required for a migration",
		Long:         "Create sql and yaml files required for a migration",
		Example:      migrateCreateCmdExamples,
		SilenceUsage: true,
		Args:         cobra.ExactArgs(1),
		PreRunE: func(cmd *cobra.Command, args []string) error {
			ec.Viper = v
			return ec.Validate()
		},
		RunE: func(cmd *cobra.Command, args []string) error {
			opts.name = args[0]
			opts.EC.Spin("Creating migration files...")
			version, err := opts.run()
			opts.EC.Spinner.Stop()
			if err != nil {
				return err
			}
Repository: hasura/graphql-engine
File: cli/migrate/migrate.go (Go)

Code before:

// unlockErr calls unlock and returns a combined error
// if a prevErr is not nil.
func (m *Migrate) unlockErr(prevErr error) error {
	if err := m.unlock(); err != nil {
		return NewMultiError(prevErr, err)
	}
	return prevErr
}

func (m *Migrate) addDryRun(ret <-chan interface{}) error {
	var lastInsertVersion int64
	for r := range ret {
		if m.stop() {
			return nil
		}

		switch r.(type) {
		case error:
			return r.(error)
		case *Migration:
			migr := r.(*Migration)
			if migr.Body != nil {
				version := int64(migr.Version)
				fileName := migr.Identifier
				fileType := convertBool(s.HasSuffix(fileName, "down.yaml"))
				if version != lastInsertVersion {
					status := printDryRunStatus(version, fileName, fileType)
					fmt.Println(status.String())
					lastInsertVersion = version
				}
			}
		}
	}
	return nil
}

func printDryRunStatus(v int64, n, t string) *bytes.Buffer {
	out := new(tabwriter.Writer)
	buf := &bytes.Buffer{}
	out.Init(buf, 0, 8, 2, ' ', 0)
	w := util.NewPrefixWriter(out)
	w.Write(util.LEVEL_0, "VERSION\tType\tName\n")
	w.Write(util.LEVEL_0, "%d\t%s\t%s\n",
		v, t, n)
	out.Flush()
	return buf
}

func convertBool(ok bool) string {
	switch ok {

Review comment (suggestion):
fileName can contain down.sql too. 

```golang
if migr.Version == migr.TargetVersion {
    return "up"
} else {
    return "down"
}
```

Code after:
		} else if err != nil {
			return nil, err

		} else {
			// create migration from up source
			migr, err = NewMigration(r, identifier, version, targetVersion, "meta", fileName)
			if err != nil {
				return nil, err
			}
		}

	} else {
		r, identifier, fileName, err := m.sourceDrv.ReadMetaDown(version)
		if os.IsNotExist(err) {
			// create "empty" migration
			migr, err = NewMigration(nil, "", version, targetVersion, "meta", "")
			if err != nil {
				return nil, err
			}

		} else if err != nil {
			return nil, err

		} else {
			// create migration from down source
			migr, err = NewMigration(r, identifier, version, targetVersion, "meta", fileName)
			if err != nil {
				return nil, err
			}
		}
	}

	if m.PrefetchMigrations > 0 && migr.Body != nil {
		//m.logVerbosePrintf("Start buffering %v\n", migr.LogString())
	} else {
		//m.logVerbosePrintf("Scheduled %v\n", migr.LogString())
	}

	return migr, nil
}

// stop returns true if no more migrations should be run against the database
// because a stop signal was received on the GracefulStop channel.
// Calls are cheap and this function is not blocking.
func (m *Migrate) stop() bool {
	if m.isGracefulStop {
		return true
	}

	select {
	case <-m.GracefulStop:
Repository: hasura/graphql-engine
File: cli/cli.go (Go)

Code before:
// through viper.
func (ec *ExecutionContext) readConfig() error {
	// need to get existing viper because https://github.com/spf13/viper/issues/233
	v := ec.Viper
	v.SetEnvPrefix("HASURA_GRAPHQL")
	v.AutomaticEnv()
	v.SetConfigName("config")
	v.SetDefault("endpoint", "http://localhost:8080")
	v.SetDefault("admin_secret", "")
	v.SetDefault("access_key", "")
	v.AddConfigPath(ec.ExecutionDirectory)
	err := v.ReadInConfig()
	if err != nil {
		return errors.Wrap(err, "cannot read config from file/env")
	}
	adminSecret := v.GetString("admin_secret")
	if adminSecret == "" {
		adminSecret = v.GetString("access_key")
	}
	ec.ServerConfig = &ServerConfig{
		Endpoint:    v.GetString("endpoint"),
		AdminSecret: adminSecret,
	}

	// check environment
	val := v.GetString("HASURA_GRAPHQL_CLI_ENVIRONMENT")
	if  val == "server-on-docker"{
		ec.isRunningOnDocker = true
	}
	return ec.ServerConfig.ParseEndpoint()
}

// setupSpinner creates a default spinner if the context does not already have
// one.
func (ec *ExecutionContext) setupSpinner() {
	if ec.Spinner == nil {
		spnr := spinner.New(spinner.CharSets[7], 100*time.Millisecond)
		spnr.Writer = os.Stderr
		ec.Spinner = spnr
	}
}

// Spin stops any existing spinner and starts a new one with the given message.
func (ec *ExecutionContext) Spin(message string) {
	if !ec.isRunningOnDocker {
		ec.Spinner.Stop()
		ec.Spinner.Prefix = message
		ec.Spinner.Start()
	}
	ec.Logger.Print(message)
}

Review comment (suggestion):
```suggestion
	val := v.GetString("cli_environment")
```

Code after:
	return nil
}

// readConfig reads the configuration from config file, flags and env vars,
// through viper.
func (ec *ExecutionContext) readConfig() error {
	// need to get existing viper because https://github.com/spf13/viper/issues/233
	v := ec.Viper
	v.SetEnvPrefix("HASURA_GRAPHQL")
	v.AutomaticEnv()
	v.SetConfigName("config")
	v.SetDefault("endpoint", "http://localhost:8080")
	v.SetDefault("admin_secret", "")
	v.SetDefault("access_key", "")
	v.AddConfigPath(ec.ExecutionDirectory)
	err := v.ReadInConfig()
	if err != nil {
		return errors.Wrap(err, "cannot read config from file/env")
	}
	adminSecret := v.GetString("admin_secret")
	if adminSecret == "" {
		adminSecret = v.GetString("access_key")
	}
	ec.ServerConfig = &ServerConfig{
		Endpoint:    v.GetString("endpoint"),
		AdminSecret: adminSecret,
	}
	return ec.ServerConfig.ParseEndpoint()
}

// setupSpinner creates a default spinner if the context does not already have
// one.
func (ec *ExecutionContext) setupSpinner() {
	if ec.Spinner == nil {
		spnr := spinner.New(spinner.CharSets[7], 100*time.Millisecond)
		spnr.Writer = os.Stderr
		ec.Spinner = spnr
	}
}

// Spin stops any existing spinner and starts a new one with the given message.
func (ec *ExecutionContext) Spin(message string) {
	if ec.IsTerminal {
		ec.Spinner.Stop()
		ec.Spinner.Prefix = message
		ec.Spinner.Start()
	} else {
		ec.Logger.Println(message)
	}
}
Repository: hasura/graphql-engine
File: cli/commands/console.go (Go)

Code before:

	// Create WaitGroup for running 3 servers
	wg := &sync.WaitGroup{}
	o.WG = wg
	wg.Add(1)
	go func() {
		err = r.router.Run(o.Address + ":" + o.APIPort)
		if err != nil {
			o.EC.Logger.WithError(err).Errorf("error listening on port %s", o.APIPort)
		}
		wg.Done()
	}()
	wg.Add(1)
	go func() {
		err = consoleRouter.Run(o.Address + ":" + o.ConsolePort)
		if err != nil {
			o.EC.Logger.WithError(err).Errorf("error listening on port %s", o.ConsolePort)
		}
		wg.Done()
	}()

	consoleURL := fmt.Sprintf("http://%s:%s/", o.Address, o.ConsolePort)

	if !o.DontOpenBrowser {
		if o.Browser != "" {
			o.EC.Spin((color.CyanString("Opening console on: %s", o.Browser)))
			defer o.EC.Spinner.Stop()
			err = open.RunWith(consoleURL, o.Browser)
			if err != nil {
				o.EC.Logger.WithError(err).Warn("Error opening browser, try to open the url with another browser")
				return err
			}
		} else {
			o.EC.Spin(color.CyanString("Opening console using default browser..."))
			defer o.EC.Spinner.Stop()

			err = open.Run(consoleURL)
			if err != nil {
				o.EC.Logger.WithError(err).Warn("Error opening browser, try to open the url manually?")
			}
		}
	}

	o.EC.Spinner.Stop()
	if o.Browser != "" {
		log.Infof("console running at: %s on %s", consoleURL, o.Browser)
	} else {
		log.Infof("console running at: %s", consoleURL)
	}

	o.EC.Telemetry.Beam()

Review comment (suggestion):
```suggestion
			o.EC.Spin(color.CyanString("Opening console on: %s", o.Browser))
```

Code after:

	// Create WaitGroup for running 3 servers
	wg := &sync.WaitGroup{}
	o.WG = wg
	wg.Add(1)
	go func() {
		err = r.router.Run(o.Address + ":" + o.APIPort)
		if err != nil {
			o.EC.Logger.WithError(err).Errorf("error listening on port %s", o.APIPort)
		}
		wg.Done()
	}()
	wg.Add(1)
	go func() {
		err = consoleRouter.Run(o.Address + ":" + o.ConsolePort)
		if err != nil {
			o.EC.Logger.WithError(err).Errorf("error listening on port %s", o.ConsolePort)
		}
		wg.Done()
	}()

	consoleURL := fmt.Sprintf("http://%s:%s/", o.Address, o.ConsolePort)

	if !o.DontOpenBrowser {
		if o.Browser != "" {
			o.EC.Spin(color.CyanString("Opening console on: %s", o.Browser))
			defer o.EC.Spinner.Stop()
			err = open.RunWith(consoleURL, o.Browser)
			if err != nil {
				o.EC.Logger.WithError(err).Warnf("failed opening console in '%s', try to open the url manually", o.Browser)
			}
		} else {
			o.EC.Spin(color.CyanString("Opening console using default browser..."))
			defer o.EC.Spinner.Stop()

			err = open.Run(consoleURL)
			if err != nil {
				o.EC.Logger.WithError(err).Warn("Error opening browser, try to open the url manually?")
			}
		}
	}

	o.EC.Spinner.Stop()
	log.Infof("console running at: %s", consoleURL)

	o.EC.Telemetry.Beam()

	wg.Wait()
	return nil
}
Repository: hasura/graphql-engine
File: cli/commands/console.go (Go)

Code before:
	wg.Add(1)
	go func() {
		err = r.router.Run(o.Address + ":" + o.APIPort)
		if err != nil {
			o.EC.Logger.WithError(err).Errorf("error listening on port %s", o.APIPort)
		}
		wg.Done()
	}()
	wg.Add(1)
	go func() {
		err = consoleRouter.Run(o.Address + ":" + o.ConsolePort)
		if err != nil {
			o.EC.Logger.WithError(err).Errorf("error listening on port %s", o.ConsolePort)
		}
		wg.Done()
	}()

	consoleURL := fmt.Sprintf("http://%s:%s/", o.Address, o.ConsolePort)

	if !o.DontOpenBrowser {
		if o.Browser != "" {
			o.EC.Spin((color.CyanString("Opening console on: %s", o.Browser)))
			defer o.EC.Spinner.Stop()
			err = open.RunWith(consoleURL, o.Browser)
			if err != nil {
				o.EC.Logger.WithError(err).Warn("Error opening browser, try to open the url with another browser")
				return err
			}
		} else {
			o.EC.Spin(color.CyanString("Opening console using default browser..."))
			defer o.EC.Spinner.Stop()

			err = open.Run(consoleURL)
			if err != nil {
				o.EC.Logger.WithError(err).Warn("Error opening browser, try to open the url manually?")
			}
		}
	}

	o.EC.Spinner.Stop()
	if o.Browser != "" {
		log.Infof("console running at: %s on %s", consoleURL, o.Browser)
	} else {
		log.Infof("console running at: %s", consoleURL)
	}

	o.EC.Telemetry.Beam()

	wg.Wait()
	return nil
}

Review comment (bug):
```suggestion
				o.EC.Logger.WithError(err).Warnf("failed opening console in '%s', try to open the url manually?", o.Browser)
```

Code after:
	wg.Add(1)
	go func() {
		err = r.router.Run(o.Address + ":" + o.APIPort)
		if err != nil {
			o.EC.Logger.WithError(err).Errorf("error listening on port %s", o.APIPort)
		}
		wg.Done()
	}()
	wg.Add(1)
	go func() {
		err = consoleRouter.Run(o.Address + ":" + o.ConsolePort)
		if err != nil {
			o.EC.Logger.WithError(err).Errorf("error listening on port %s", o.ConsolePort)
		}
		wg.Done()
	}()

	consoleURL := fmt.Sprintf("http://%s:%s/", o.Address, o.ConsolePort)

	if !o.DontOpenBrowser {
		if o.Browser != "" {
			o.EC.Spin(color.CyanString("Opening console on: %s", o.Browser))
			defer o.EC.Spinner.Stop()
			err = open.RunWith(consoleURL, o.Browser)
			if err != nil {
				o.EC.Logger.WithError(err).Warnf("failed opening console in '%s', try to open the url manually", o.Browser)
			}
		} else {
			o.EC.Spin(color.CyanString("Opening console using default browser..."))
			defer o.EC.Spinner.Stop()

			err = open.Run(consoleURL)
			if err != nil {
				o.EC.Logger.WithError(err).Warn("Error opening browser, try to open the url manually?")
			}
		}
	}

	o.EC.Spinner.Stop()
	log.Infof("console running at: %s", consoleURL)

	o.EC.Telemetry.Beam()

	wg.Wait()
	return nil
}

type cRouter struct {
	router  *gin.Engine
	migrate *migrate.Migrate
}
Repository: hasura/graphql-engine
File: cli/commands/console.go (Go)

Code before:
	if !o.DontOpenBrowser {
		if o.Browser != "" {
			o.EC.Spin((color.CyanString("Opening console on: %s", o.Browser)))
			defer o.EC.Spinner.Stop()
			err = open.RunWith(consoleURL, o.Browser)
			if err != nil {
				o.EC.Logger.WithError(err).Warn("Error opening browser, try to open the url with another browser")
				return err
			}
		} else {
			o.EC.Spin(color.CyanString("Opening console using default browser..."))
			defer o.EC.Spinner.Stop()

			err = open.Run(consoleURL)
			if err != nil {
				o.EC.Logger.WithError(err).Warn("Error opening browser, try to open the url manually?")
			}
		}
	}

	o.EC.Spinner.Stop()
	if o.Browser != "" {
		log.Infof("console running at: %s on %s", consoleURL, o.Browser)
	} else {
		log.Infof("console running at: %s", consoleURL)
	}

	o.EC.Telemetry.Beam()

	wg.Wait()
	return nil
}

type cRouter struct {
	router  *gin.Engine
	migrate *migrate.Migrate
}

func (r *cRouter) setRoutes(migrationDir, metadataFile string, logger *logrus.Logger) {
	apis := r.router.Group("/apis")
	{
		apis.Use(setLogger(logger))
		apis.Use(setFilePath(migrationDir))
		apis.Use(setMigrate(r.migrate))
		// Migrate api endpoints and middleware
		migrateAPIs := apis.Group("/migrate")
		{
			settingsAPIs := migrateAPIs.Group("/settings")
			{
				settingsAPIs.Any("", api.SettingsAPI)
			}

Review comment (suggestion):
We don't need to do this here since console is running at this point irrespective of the browser flag.

Change it to just
```go
log.Infof("console running at: %s", consoleURL)
```

Code after:
	if !o.DontOpenBrowser {
		if o.Browser != "" {
			o.EC.Spin(color.CyanString("Opening console on: %s", o.Browser))
			defer o.EC.Spinner.Stop()
			err = open.RunWith(consoleURL, o.Browser)
			if err != nil {
				o.EC.Logger.WithError(err).Warnf("failed opening console in '%s', try to open the url manually", o.Browser)
			}
		} else {
			o.EC.Spin(color.CyanString("Opening console using default browser..."))
			defer o.EC.Spinner.Stop()

			err = open.Run(consoleURL)
			if err != nil {
				o.EC.Logger.WithError(err).Warn("Error opening browser, try to open the url manually?")
			}
		}
	}

	o.EC.Spinner.Stop()
	log.Infof("console running at: %s", consoleURL)

	o.EC.Telemetry.Beam()

	wg.Wait()
	return nil
}

type cRouter struct {
	router  *gin.Engine
	migrate *migrate.Migrate
}

func (r *cRouter) setRoutes(migrationDir, metadataFile string, logger *logrus.Logger) {
	apis := r.router.Group("/apis")
	{
		apis.Use(setLogger(logger))
		apis.Use(setFilePath(migrationDir))
		apis.Use(setMigrate(r.migrate))
		// Migrate api endpoints and middleware
		migrateAPIs := apis.Group("/migrate")
		{
			settingsAPIs := migrateAPIs.Group("/settings")
			{
				settingsAPIs.Any("", api.SettingsAPI)
			}
			squashAPIs := migrateAPIs.Group("/squash")
			{
				squashAPIs.POST("/create", api.SquashCreateAPI)
				squashAPIs.POST("/delete", api.SquashDeleteAPI)
			}
Repository: hasura/graphql-engine
File: console/src/components/Services/About/About.js (JavaScript)

Code before:
import styles from './About.scss';

class About extends Component {
  state = {
    serverVersion: null,
    latestServerVersion: null,
    consoleAssetVersion: globals.consoleAssetVersion,
  };

  componentDidMount() {
    fetch(Endpoints.version)
      .then(response => response.json())
      .then(serverVersion =>
        this.setState({
          serverVersion: serverVersion.version,
        })
      );

    fetch(Endpoints.updateCheck)
      .then(response => response.json())
      .then(latest =>
        this.setState({
          latestServerVersion: latest.latest,
        })
      );
    fetch('http://localhost:8080/v1/query', {
      method: 'POST',
      headers: {
        'Content-Type': 'application/json',
      },
      body: JSON.stringify({
        type: 'run_sql',
        args: {
          sql: 'SELECT version();',
        },
      }),
    })
      .then(response => response.json())
      .then(data =>
        this.setState({
          pgVersion: data.result[1],
        })
      );
  }

  render() {
    const {
      serverVersion,
      latestServerVersion,
      consoleAssetVersion,
      pgVersion,

Review comment (suggestion):
- Add an empty line above this for readability
- The endpoint should be the variable `Endpoints.query` instead of `localhost...`

Code after:
import styles from './About.scss';

class About extends Component {
  state = {
    serverVersion: null,
    latestServerVersion: null,
    consoleAssetVersion: globals.consoleAssetVersion,
    pgVersion: null,
  };

  componentDidMount() {
    fetch(Endpoints.version)
      .then(response => response.json())
      .then(serverVersion =>
        this.setState({
          serverVersion: serverVersion.version,
        })
      );

    fetch(Endpoints.updateCheck)
      .then(response => response.json())
      .then(latest =>
        this.setState({
          latestServerVersion: latest.latest,
        })
      );

    fetch(Endpoints.query, {
      method: 'POST',
      headers: {
        'Content-Type': 'application/json',
      },
      body: JSON.stringify({
        type: 'run_sql',
        args: {
          sql: 'SELECT version();',
        },
      }),
    })
      .then(response => response.json())
      .then(data =>
        this.setState({
          pgVersion: data.result[1],
        })
      );
  }

  render() {
    const {
      serverVersion,
      latestServerVersion,
Repository: hasura/graphql-engine
File: console/src/components/Services/About/About.js (JavaScript)

Code before:

    fetch(Endpoints.updateCheck)
      .then(response => response.json())
      .then(latest =>
        this.setState({
          latestServerVersion: latest.latest,
        })
      );
    fetch('http://localhost:8080/v1/query', {
      method: 'POST',
      headers: {
        'Content-Type': 'application/json',
      },
      body: JSON.stringify({
        type: 'run_sql',
        args: {
          sql: 'SELECT version();',
        },
      }),
    })
      .then(response => response.json())
      .then(data =>
        this.setState({
          pgVersion: data.result[1],
        })
      );
  }

  render() {
    const {
      serverVersion,
      latestServerVersion,
      consoleAssetVersion,
      pgVersion,
    } = this.state;

    const spinner = <i className="fa fa-spinner fa-spin" />;

    const getServerVersionSection = () => {
      return (
        <div>
          <b>Server version: </b>
          <span className={styles.add_mar_left_mid}>
            {serverVersion || spinner}
          </span>
        </div>
      );
    };

    const getLatestServerVersionSection = () => {
      let updateLinks;

Review comment (suggestion):
Initialize `pgVersion` value in state. Set `pgVersion: null` after line 14. 

Code after:
      );

    fetch(Endpoints.updateCheck)
      .then(response => response.json())
      .then(latest =>
        this.setState({
          latestServerVersion: latest.latest,
        })
      );

    fetch(Endpoints.query, {
      method: 'POST',
      headers: {
        'Content-Type': 'application/json',
      },
      body: JSON.stringify({
        type: 'run_sql',
        args: {
          sql: 'SELECT version();',
        },
      }),
    })
      .then(response => response.json())
      .then(data =>
        this.setState({
          pgVersion: data.result[1],
        })
      );
  }

  render() {
    const {
      serverVersion,
      latestServerVersion,
      consoleAssetVersion,
      pgVersion,
    } = this.state;

    const spinner = <i className="fa fa-spinner fa-spin" />;

    const getServerVersionSection = () => {
      return (
        <div>
          <b>Server version: </b>
          <span className={styles.add_mar_left_mid}>
            {serverVersion || spinner}
          </span>
        </div>
      );
    };