-
Notifications
You must be signed in to change notification settings - Fork 322
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
fix(warehouse): increase async job timeout #2721
Conversation
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.
LGTM
@@ -91,5 +91,5 @@ const ( | |||
MaxQueryRetries int = 3 | |||
RetryTimeInterval = 10 * time.Second | |||
MaxAttemptsPerJob int = 3 | |||
WhAsyncJobTimeOut = 10 * time.Second |
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.
Should we make this configurable? Is this the default value we are changing?
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.
Added these.
Codecov ReportBase: 46.38% // Head: 46.80% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #2721 +/- ##
==========================================
+ Coverage 46.38% 46.80% +0.41%
==========================================
Files 295 297 +2
Lines 48610 48696 +86
==========================================
+ Hits 22550 22791 +241
+ Misses 24646 24477 -169
- Partials 1414 1428 +14
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
func (asyncWhJob *AsyncJobWhT) AddWarehouseJobHandler(w http.ResponseWriter, r *http.Request) { | ||
pkgLogger.Info("[WH-Jobs] Got Async Job Add Request") | ||
pkgLogger.LogRequest(r) | ||
func (a *AsyncJobWhT) AddWarehouseJobHandler(w http.ResponseWriter, r *http.Request) { |
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.
a
looks like too generic. rename to asyncJob
??
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.
LGTM!
Description
AsyncJobWhT
as a instead of asyncWhJob.AsyncJobWhT
itself.Notion Ticket
https://www.notion.so/rudderstacks/Warehouse-Async-Job-Framework-Sync-79a575f429e540d79e9094651b30d7e8
Security