-
Notifications
You must be signed in to change notification settings - Fork 0
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
web plugin, refactoring #15
Conversation
plugins/web/plugin.go
Outdated
errorText := fmt.Sprintf("Invalid input data: %s", string(input)) | ||
return errors.New(errorText) | ||
} | ||
p.storage.Mutex.Lock() |
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.
торчащий наружу мьютекс – плохая идея
plugins/web/plugin.go
Outdated
} | ||
for pos, element := range p.storage.Data[unsubscribeData.EventName] { | ||
if element == unsubscribeData.Url { | ||
p.storage.Data[unsubscribeData.EventName][pos] = p.storage.Data[unsubscribeData.EventName][len(p.storage.Data[unsubscribeData.EventName])-1] |
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.
не понял, что тут происходит
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.
громоздко удаляю подписчика
plugins/web/plugin.go
Outdated
log.Error(err) | ||
continue | ||
} | ||
req.Header.Set("Content-Type", "application/json") |
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.
мы уверены, что нам будут только json посылать?
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.
как минимум сейчас я бы говорил только про json
plugins/web/plugin.go
Outdated
|
||
func (p PluginWeb) ProcessEvent(eventData event.Event) { | ||
httpClient := &http.Client{} |
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://medium.com/@nate510/don-t-use-go-s-default-http-client-4804cb19f779#.uba9lfk7b
plugins/web/plugin.go
Outdated
func (p PluginWeb) ProcessEvent(eventData event.Event) { | ||
httpClient := &http.Client{} | ||
for _, subscribeUrl := range p.storage.Data[eventData.Name] { | ||
req, err := http.NewRequest("POST", subscribeUrl, bytes.NewBuffer(eventData.Payload)) |
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.
реквест можно создать вне цикла, а внутри только урл менять. ну и все содержимое цикла в горутину кинуть
dispatcher/dispatcher_test.go
Outdated
subscrService.ProcessSubscribe(subscribeData) | ||
client := &FakePostClient{t, requestUrl, requestData} | ||
dsp := NewDispatcher(rmq, storageUnit, client) | ||
subscribeData := []byte("{\"test\": \"hello\"}") |
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.
FYI, такое можно в бэктиках писать
[]byte(`{"test": "hello"}`)
plugins/web/plugin.go
Outdated
log.Error(err) | ||
continue | ||
} | ||
resp, err := p.client.Do(req) |
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.
в горутину не стал уводить?
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.
забыл)
plugins/web/plugin.go
Outdated
continue | ||
} | ||
// TODO: add ability to log statistics | ||
go p.client.Do(req) |
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.
Я тут кстати не обломаюсь с тем, что реквест может поменяться до того, как отправится? Может стоит каждый раз все же новый реквест создавать?
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.
может. чтоб не обломаться, можно сделать так:
go func(req http.Request, url string) {
req.URL, err = url.Parse(subscribeUrl)
if err != nil {
log.Error(err)
continue
}
// TODO: add ability to log statistics
go p.client.Do(req)
} (&req, subscribeUrl)
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.
ну или вынести это в отдельный метод
No description provided.