Skip to content

Conversation

@grphil
Copy link
Collaborator

@grphil grphil commented Feb 7, 2025

No description provided.

@misha567889 misha567889 self-requested a review February 10, 2025 15:22
func (r *ResponseFiles) Get(fileName string) (string, bool) {
for _, f := range r.fileNames {
if fileName == f {
return filepath.Join(r.R.BaseFolder, r.fileNames[0]), true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тут точно fileNames[0]?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Справедливо


func fillInInvokerConfig(config *InvokerConfig) {
if config.QueueSize == nil {
config.QueueSize = pointer.Int(10)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

в константу бы

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

хз на самом деле, тут по сути все эти fillIN это одна большая константная функция, там тогда константы на каждое поле надо делать и читать это будет невозможно. Тут как раз удобно что ты быстренько на код посмотрел и понял какие дефолт значения


func RespOK(c *gin.Context, data gin.H) {
c.JSON(http.StatusOK, gin.H{
"ok": true,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а зачем передавать этот ок, если есть статус код?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А это вообще я потом поменяю на другую схему, через несколько дней напишу свое видиние нашей отказоучтойчивости и будем это обсуждать

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ну и там вообще все полностью поменяется потом в этом респонзе

// For uploads, FilePath or FilePaths should be specified (depending on whether the resource is single-file or not).
// Filename will be taken from filename inside the path.
FilePath string `json:"-"`
FilePaths []string `json:"-"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

почему не хранить всегда []string?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это вообще сильно поменяю в следующем коммите, пока можно забить

package invokerconn

import (
models2 "testing_system/common/db/models"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

почему 2

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

О, надо пофиксить, там вообще все структуры сломались в какой-то момент и я не везде починил

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

я ничего не понял) какой-то генератор ключа, в cacheKey хранится три штуки, но всегда используются <= 2
что тут вообще происходит

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Дописал пояснение в комментах


// This will be changed in later commits

const (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

вообще мне кажется, что можно для "енумов" свои типы делать отдельные, чтобы не передать что-то не то (вот как в storageconn/structs.go)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ага, так и будет в следующем коммите

@grphil grphil merged commit 22fc466 into main Feb 15, 2025
1 check passed
@grphil grphil deleted the invoker branch February 15, 2025 19:48
@grphil grphil restored the invoker branch February 15, 2025 19:53
@grphil grphil deleted the invoker branch February 15, 2025 19:54
@grphil grphil restored the invoker branch February 15, 2025 19:54
@grphil grphil deleted the invoker branch February 15, 2025 19:56
Artyom321 pushed a commit that referenced this pull request Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants