fix(warehouse): syncs issues #2732
Conversation
proto/warehouse/warehouse.proto
Outdated
| @@ -73,6 +73,7 @@ message WHUploadResponse { | |||
| int32 duration = 14; | |||
| repeated WHTable tables = 15; | |||
| bool isArchivedUpload = 16; | |||
| int32 status_code = 17; | |||
There was a problem hiding this comment.
I am a bit concern with adding HTTP semantics into GRPC, is it because we are using an HTTP API at some part of the chain ?
There was a problem hiding this comment.
I followed the GRPC documentation, Currently, there is no way to send StatusNoContent (204).
grpc-ecosystem/grpc-gateway#240
There was a problem hiding this comment.
List of HTTP status codes supported now.
There was a problem hiding this comment.
After a discussion with @abhimanyubabbar, From an API perspective, the status code should be 404 instead of 204.
There was a problem hiding this comment.
Although, in both of the scenarios the customer is looking for:
- Mergeable sync which now doesn't exist anymore.
- Unintended sync
We should probably return Resource Not Found.
Codecov ReportBase: 46.71% // Head: 46.88% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #2732 +/- ##
==========================================
+ Coverage 46.71% 46.88% +0.16%
==========================================
Files 298 298
Lines 48883 48934 +51
==========================================
+ Hits 22837 22942 +105
+ Misses 24597 24537 -60
- Partials 1449 1455 +6
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. |
achettyiitr
force-pushed
the
fix.warehouse-syncs
branch
from
f818e88 to
b182542
Compare
warehouse/api.go
Outdated
| @@ -106,6 +110,7 @@ const ( | |||
| TriggeredSuccessfully = "Triggered successfully" | |||
| NoPendingEvents = "No pending events to sync for this destination" | |||
| DownloadFileNamePattern = "downloadfile.*.tmp" | |||
| NoSuchSyncs = "No such sync exist" | |||
Description
Notion Ticket
https://www.notion.so/rudderstacks/Syncs-dashboard-fixes-4d3c454856d844e09cc15a73012a37e1
Security