CXXCBC-598: Columnar - propagate bootstrap errors to HTTP operations#666
Merged
thejcfactor merged 2 commits intocouchbase:mainfrom Oct 2, 2024
Merged
CXXCBC-598: Columnar - propagate bootstrap errors to HTTP operations#666thejcfactor merged 2 commits intocouchbase:mainfrom
thejcfactor merged 2 commits intocouchbase:mainfrom
Conversation
thejcfactor
force-pushed
the
CXXCBC-598
branch
from
25568fb to
f8ae7aa
Compare
Changes ======= * Create and error_union type of std::variant<std::monostate, std::error_code, impl::bootstrap_error> that can be passed around to Columnar callbacks * Allow the http_session_manager drain queue to accept an error_union * Update error message in certain contexts to indicate a bootstrap error is the reason the HTTP operation failed. * Confirm columnar testing passes
thejcfactor
force-pushed
the
CXXCBC-598
branch
from
f8ae7aa to
f4ea5be
Compare
thejcfactor
commented
Oct 1, 2024
avsej
approved these changes
Oct 2, 2024
DemetrisChr
approved these changes
Oct 2, 2024
Contributor
DemetrisChr
left a comment
DemetrisChr
left a comment
There was a problem hiding this comment.
LGTM
I think eventually we want to use a non-columnar equivalent of couchbase::core::columnar::error in the HTTP component & when draining the deferred queue in the session manager (and in the core API more broadly). That might allow us to remove the error union from the HTTP operation signature in the agent, which could be cleaner. But that's something to consider later on as it would be a bigger change.
thejcfactor
force-pushed
the
CXXCBC-598
branch
from
176d2bd to
8c5d74f
Compare
Contributor
Author
|
Yes! I agree. At first I tried to work through a more generic approach, but realized how pervasive the change would need to be so I reverted to something I thought would be more pointed and less evasive. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes