[source-recharge] - Migrate to CDK v3.7.0 #42076
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
There was a problem hiding this comment.
Just one comment, but not a blocker on merging, just a thought that we want to take note of. If we keep running into situations where we need to reimplement HttpStatusErrorHandler, it means we may need to add new functionality. I've noticed this on a couple version bump this week
| def interpret_response(self, response_or_exception: Optional[Union[Response, Exception]] = None) -> ErrorResolution: | ||
|
|
||
| if isinstance(response_or_exception, Response): | ||
| content_length = int(response_or_exception.headers.get("Content-Length", 0)) |
There was a problem hiding this comment.
Seems like we need this custom handler, but can you add a comment about why we need the custom component. Basically that we need to inspect the header.
I think like its a feature gap that our http client isn't able to inspect the response headers and decide things on a predicate. The lines between what the declarative component vs the http client are starting to get a little blurry, but the more we see us having to implement a subclass of HttpStatusErrorHandler, the more it signals that it's lacking features.
There was a problem hiding this comment.
Yeah -- I've noticed a couple on-off checks in the existing backoff_time or should_retry methods. I feel like there hasn't been a consistent trend between these though.
What
should_retryfrom streamget_error_handlermethodRechargeErrorHandlershould_retrytestHow
Review guide
streams.pyrecharge_error_handler.pyUser Impact
Can this PR be safely reverted and rolled back?