Improve error messages for concurrent CDK #34754
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
There was a problem hiding this comment.
I think this is definitely an improvement, and as such can be merged — I suggest that we figure out how we want folks to contact us, and state that more specifically.
I think issues are fine as we will get a triage process in place. The likelihood that we will be able to help in GH issues is higher than on Slack. Right?
| # We do not expect this error to happen. The futures created during concurrent syncs should catch the exception and | ||
| # push it to the queue. If this exception occurs, please review the futures and how they handle exceptions. | ||
| self._most_recently_seen_exception = RuntimeError( | ||
| f"Failed processing a future: {optional_exception}. Please contact the Airbyte team." |
There was a problem hiding this comment.
Let's clarify how the users should contact our team. If this situation is a bug and/or requires work from the CDK team, we should ask folks to file an issue on airbytehq/airbyte, right?
There was a problem hiding this comment.
I can't confirm as I'm not sure what is our official support process. Maybe @girarda can chime in?
EDIT: I'll merge this change regardless as it is like that elsewhere in the code. Note that this is not because I don't see this as important but I see this as a different issue. I would also like not having specific line of codes share the support process and have a clear page where this is described as this is easier to maintain.
| # If we track state value using records cursor field, we can only do that if there is one partition. This is because we save | ||
| # the state every time we close a partition. We assume that if there are multiple slices, they need to be providing | ||
| # boundaries. There are cases where partitions could not have boundaries: | ||
| # * The cursor should be per-partition | ||
| # * The stream state is actually the parent stream state | ||
| # There might be other cases not listed above. Those are not supported today hence the stream should not use this cursor for | ||
| # state management. For the specific user that was affected with this issue, we need to: | ||
| # * Fix state tracking (which is currently broken) | ||
| # * Make the new version available | ||
| # * (Probably) ask the user to reset the stream to avoid data loss |
There was a problem hiding this comment.
Thank you for the details here!
maxi297
deleted the
maxi297/update-documentation-and-error-messages-for-concurrent
branch
What
Adding documentation through comments and improving error messages