increment 13 #13
increment 13 #13
Conversation
| func sendBulk(c *http.Client, addr string, mm []queuedMetric) error { | ||
| b := zipBulk(mm) | ||
|
|
||
| req, err := http.NewRequest(http.MethodPost, endpointBulk(addr), b) |
There was a problem hiding this comment.
тут немного не понял, можно в клиенте сразу выделить нужные методы и их использовать. А то в проекте есть клиент, а запросы создаются через http
|
|
||
| req, err := http.NewRequest(http.MethodPost, endpointBulk(addr), b) | ||
| if err != nil { | ||
| panic(err) |
There was a problem hiding this comment.
а зачем паниковать, если есть возврат ошибки? потом эту панику придется еще отлавливать и логировать
| @@ -24,14 +24,17 @@ func UpdateHandleFunc(st storage.Storage) func(http.ResponseWriter, *http.Reques | |||
| return | |||
| } | |||
|
|
|||
| if err := st.Update(chi.URLParam(r, "name"), m); err != nil { | |||
| if err := st.Update(r.Context(), metrics.Named{ | |||
There was a problem hiding this comment.
бывает полезно передавать context.WithTimeout, чтобы не ждать бесконечно ответа от вложенного вызова
| @@ -2,11 +2,12 @@ package httpclient | |||
|
|
|||
| import ( | |||
| "net/http" | |||
| "time" | |||
|
|
|||
| "github.com/go-resty/resty/v2" | |||
| ) | |||
|
|
|||
| func New() *http.Client { | |||
There was a problem hiding this comment.
в клиенте можно написать методы помощники и их использовать с выделением интерфейса
|
|
||
| var ( | ||
| gaugeInsert = fmt.Sprintf("INSERT INTO %s(name, value) VALUES ($1, $2) ON CONFLICT (name) DO UPDATE SET value = EXCLUDED.value", gaugesTable) | ||
| counterInsert = fmt.Sprintf("INSERT INTO %s(name, value) VALUES ($1, $2) ON CONFLICT (name) DO UPDATE SET value = %s.value + EXCLUDED.value", countersTable, countersTable) |
There was a problem hiding this comment.
это верное решение, вынести запросы в константы, чтобы не захламлять код самого метода
| *sql.DB | ||
| } | ||
|
|
||
| func newDB(dsn string) (*db, error) { |
There was a problem hiding this comment.
а почему конструктор приватный?
There was a problem hiding this comment.
А он не используется снаружи ;)
| } | ||
|
|
||
| func newDB(dsn string) (*db, error) { | ||
| src, err := iofs.New(fs, "migrations") |
There was a problem hiding this comment.
можно попробовать миграции вынести в отдельный экшен гитхаба.
чтобы пайплайн был приближен к продакт разработке.
- линтер
- сборка проекта
- запуск юнит тестов
- применение миграций
- деплоим код на сервера
Применение миграций в пайплайне обычно выносят в отдельную джобу, чтобы при необходимости можно было откатить эти миграции либо получить ошибку применения миграций и не раскатывать код на сервера. А тут в случае ошибки, код будет уже на серверах и миграция может не отработать - что тогда делать?
There was a problem hiding this comment.
5. А тут в случае ошибки, код будет уже на серверах и миграция может не отработать - что тогда делать?
Не понял вот этого момента :(
Допустим, мы написали миграции, которые проходят криво или содержат ошибку. Пытаемся катить. Канарейка, либо первый же инстанс (если деплоимся инстанс-группами), либо первый же под (если мы в k8s) не может стартовать, миграция падает с ошибкой. Остальные экземпляры бекенда продолжают работать на старой версии кода, деплой дальше не идёт, можно откатываться и фиксить миграцию.
Миграция отдельной джобой в этом сценарии ничего не меняет, зато усложняет пайплайн.
| return nil, err | ||
| } | ||
|
|
||
| database, err := sql.Open("pgx", dsn) |
There was a problem hiding this comment.
Чтобы улучшить работу с бд, необходимо выделать коннекшен пул и работать с бд через коннекшен пул. Можно почитать как это делается в документации.
There was a problem hiding this comment.
Но ведь sql стандартной библиотеки выделяет пул соединений «под капотом». Или ты имеешь в виду, что есть смысл его ограничить по количеству соединений?
| return db.DB.PingContext(ctx) | ||
| } | ||
|
|
||
| func (db *db) Get(ctx context.Context, t, name string) (metrics.Metric, error) { |
There was a problem hiding this comment.
Кажется, что интерфейс первоначального сторейджа получился не совместимым с предыдущими сторейджами. По факту должно быть так - выделен общий интерфейс для сторейджа, далее различные имплементации сторейджа реализуют этот интерфейс, а в хэндлеры прокидываются нужная реализация через интерфейс.
те должно быть несколько типов сторейджа с общим интерфейсом
- инмемори
- файл сторейдж
- дб сторейдж
а хэндлер, просто дергает нужный метод интерфейса и ничего не знает о реализации этого метода
There was a problem hiding this comment.
Но здесь ровно так и есть, просто пришлось их порефакторить, чтобы они все принимали контекст (это всё равно полезно для всех, кроме инмемори).
No description provided.