-
Notifications
You must be signed in to change notification settings - Fork 0
Hw12 13 14 15 calendar #13
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
Conversation
agneum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Спасибо за вариант решения. Хорошая работа
Итого: 8 баллов из 10. Принято
| switch conf.Logger.Output { | ||
| case "stderr": | ||
| logg = logg.Output(os.Stderr) | ||
| case "stdout": | ||
| logg = logg.Output(os.Stdout) | ||
| default: | ||
| logg = logg.Output(os.Stdout) // Default to stdout if not specified | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Конфигурацию логгера лучше сделать в конструкторе и не выносить в main
| ctx, cancel := context.WithCancel(context.Background()) | ||
| defer cancel() | ||
|
|
||
| logg := logger.New(ctx, conf.Logger.Level, nil, true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Предложил бы использовать структуру для передачи параметров в конструктор. nil и bool в качестве аргументов выглядят не очень хорошо
| for { | ||
| conn, err := listener.Accept() | ||
| if err != nil { | ||
| log.Printf("failed to accept connection: %v", err) | ||
| continue | ||
| } | ||
|
|
||
| go handleConnection(conn) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не совсем понимаю, для чего этот цикл
| Port: conf.DB.Port, | ||
| Name: conf.DB.Name, | ||
| }); err != nil { | ||
| panic(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Желательно не паниковать и корреткно обработать ошибку
| @@ -0,0 +1,67 @@ | |||
| package main | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Предложил бы использовать бинарник goose, т.к. приложение, использующее его пакеты, не добавляет специальной функциональностью. Скорее, наоборот, его функциональность ограничена
| @@ -0,0 +1,123 @@ | |||
| package envreader | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Для конфигурации приложения рекомендовал бы использовать один файл в удобном формате и любой парсер этого файла, либо специальный пакет для загрузки конфигурации
| clientIP := r.RemoteAddr | ||
| dateTime := time.Now().Format(time.RFC3339) | ||
| method := r.Method | ||
| path := r.URL.Path | ||
| httpVersion := r.Proto | ||
| userAgent := r.Header.Get("User-Agent") | ||
|
|
||
| logger.Info( | ||
| fmt.Sprintf( | ||
| "Client IP: %s, DateTime: %s, Method: %s, Path: %s, HTTP Version: %s, User Agent: %s", | ||
| clientIP, dateTime, method, path, httpVersion, userAgent, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Кажется, дублируется код middleware
| select { | ||
| case <-ctx.Done(): | ||
| return ctx.Err() | ||
| default: | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В in-memory хранилище все операции достаточно быстрые, нет взаимодействия со сторонними сервисами и т.д., поэтому использование контекста не имеет существенной роли
| func New(ctx context.Context, level string, sampler Sampler, stack bool) *Logger { | ||
| var logs Logger | ||
| logs.ctx = ctx | ||
| logs.sampler = sampler | ||
| logs.stack = stack | ||
|
|
||
| switch level { | ||
| case "trace": | ||
| logs.Level(TraceLevel) | ||
| case "debug": | ||
| logs = logs.Level(DebugLevel) | ||
| case "warn": | ||
| logs = logs.Level(WarnLevel) | ||
| case "error": | ||
| logs = logs.Level(ErrorLevel) | ||
| default: | ||
| logs = logs.Level(InfoLevel) | ||
| } | ||
|
|
||
| return &logs | ||
| } | ||
|
|
||
| func NewWriter(ctx context.Context, w io.Writer, sampler Sampler, stack bool) Logger { | ||
| if w == nil { | ||
| w = io.Discard | ||
| } | ||
| lw, ok := w.(LevelWriter) | ||
| if !ok { | ||
| lw = LevelWriterAdapter{w} | ||
| } | ||
| return Logger{w: lw, level: TraceLevel, ctx: ctx, sampler: sampler, stack: stack} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Два конструктора с разным набором аргументов выглядят необычно.
| @@ -1,20 +1,148 @@ | |||
| package logger | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Предложил бы использовать стандартный пакет для логирования https://pkg.go.dev/log
Домашнее задание №12 «Заготовка сервиса Календарь»
Критерии оценки
на пакеты по определенной логике) - до 2 баллов
Реализовано хранилище:
Зачёт от 7 баллов