Init configuration from file #3

zeripath · 2019-05-24T21:00:17Z

zeripath commented

2019-05-24 21:00:17 +00:00

This PR provides a way of configuring the loggers independently from the program.

The main benefit is that logging can be set for unit tests

This PR provides a way of configuring the loggers independently from the program. The main benefit is that logging can be set for unit tests

zeripath commented

2019-05-25 08:45:13 +00:00

OK so an example test program looking like:

package main

import "gitea.com/gitea/log"

func main() {
	defer log.Close()
	log.Trace("Trace test")
	log.Debug("Debug test")
	log.Info("Info test")
	log.Warn("Warn test")
	log.Error("Error test")
	log.Critical("Critical test")
}

with configuration:

{
    "DEFAULT_BUFFER_LEN":1000,
    "default": {
        "console": {
            "config": {
                "colorize":true, 
                "flags":"stdflags",
                "stacktraceLevel":"error"
            }
        }
    }
}

works and changes the log configuration.

Now the reason for doing config in init is that unit tests in go are quite annoying - they don't have an easy way of doing set-up consistently without requiring a TestMain for each package.

If we allow our configuration to occur in init we can always pass log configuration in to any test.

OK so an example test program looking like: ```go package main import "gitea.com/gitea/log" func main() { defer log.Close() log.Trace("Trace test") log.Debug("Debug test") log.Info("Info test") log.Warn("Warn test") log.Error("Error test") log.Critical("Critical test") } ``` with configuration: ```json { "DEFAULT_BUFFER_LEN":1000, "default": { "console": { "config": { "colorize":true, "flags":"stdflags", "stacktraceLevel":"error" } } } } ``` works and changes the log configuration. Now the reason for doing config in init is that unit tests in go are quite annoying - they don't have an easy way of doing set-up consistently without requiring a TestMain for each package. If we allow our configuration to occur in init we can always pass log configuration in to any test.

zeripath commented

2019-05-25 08:45:51 +00:00

One final issue is the requirement to add a defer log.Close() to ensure that the final logs are written. I'll look at this. - Looks like there is no other way.

One final issue is the requirement to add a `defer log.Close()` to ensure that the final logs are written. I'll look at this. - Looks like there is no other way.

lunny commented

2019-05-25 09:03:56 +00:00

Maybe we can call runtime.SetFinalizer(obj, func(obj *typeObj)) to instead defer log.Close.

Maybe we can call `runtime.SetFinalizer(obj, func(obj *typeObj))` to instead `defer log.Close`.

zeripath commented

2019-05-25 12:03:08 +00:00

hmm... so that looked like good idea but it's neither guaranteed to run on os.Exit(...) nor is it guaranteed to run for objects allocated in init().

hmm... so that looked like good idea but it's neither guaranteed to run on `os.Exit(...)` nor is it guaranteed to run for objects allocated in `init()`.

zeripath commented

2019-05-25 12:22:06 +00:00

I think we just have to tolerate that people should add a defer log.Close() to main() and suggest defer log.Flush() added to unit tests.

I think we just have to tolerate that people should add a `defer log.Close()` to `main()` and suggest `defer log.Flush()` added to unit tests.

lunny commented

2019-06-03 14:52:39 +00:00

Please resolve the conflicts

zeripath commented

2019-06-03 15:10:34 +00:00

Done.

lunny requested changes 2019-06-04 03:22:43 +00:00

Dismissed

log.go Outdated

						
				@ -173,3 +200,3 @@

				}

				// Close closes all the loggers

				// Close closes the default logger

lunny commented

2019-06-04 03:21:31 +00:00

Why not all the loggers?

zeripath commented

2019-06-05 11:36:02 +00:00

Sorry missed changing that comment. Done now.

log.go

						
				@ -230,2 +280,4 @@

					}

					// create a default log to write to console

					NewLogger(1000, "std", "console", fmt.Sprintf(`{"flags":%d, "stacktraceLevel":"error"}`,

lunny commented

2019-06-04 03:22:39 +00:00

If config load successfully, the default logger should be disabled I think.

zeripath commented

2019-06-05 11:35:31 +00:00

Take a look at line 278:

	if configFile != "" {
		err = ConfigureFromFile(configFile)
		if err == nil { 
			return // <- Line 278
		}
	}

if there's no error during configuration from the provided configuration then init() returns here.

Take a look at line 278: ```go if configFile != "" { err = ConfigureFromFile(configFile) if err == nil { return // <- Line 278 } } ``` if there's no error during configuration from the provided configuration then `init()` returns here.

lunny commented

2019-06-06 09:31:42 +00:00

OK. Just found we need add reactions on code comments.

lunny approved these changes 2019-06-06 09:31:58 +00:00

Dismissed

jonasfranz approved these changes 2019-06-08 10:11:38 +00:00

Dismissed

jonasfranz closed this pull request

2019-06-08 10:12:22 +00:00

This repo is archived. You cannot comment on pull requests.

No reviewers

No Milestone

No Assignees

3 Participants

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: gitea/log#3