Skip to content
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

Async support follow-up 2 #153

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

justinchuch
Copy link
Contributor

Revert some of the changes in #149, which are not ready to merge.

Update MongoDatabaseHanlder:

  • log unknown / unexpected error
  • enhance error handling
  • extract and unwrap CompletionException

Add method in AsyncMongoCollection and MongoCollection

Change method visibility in AbstractMongoBackend, AbstractMongoDatabase, AbstractMongoCollection, Utils to public for extensibility.

@coveralls
Copy link

coveralls commented Jul 6, 2020

Coverage Status

Coverage decreased (-0.02%) to 96.936% when pulling 8109dc3 on justinchuch:async-support-2 into 1e869b5 on bwaldvogel:master.

@justinchuch justinchuch force-pushed the async-support-2 branch 3 times, most recently from 30b0fe7 to 5ebf29a Compare July 16, 2020 15:44
@bwaldvogel
Copy link
Owner

@justinchuch: Could you indicate more clearly which part of this PR is the revert? I would prefer to see one or two commits just for the reverts.

@@ -469,7 +469,7 @@ public QueryResult handleQuery(QueryParameters queryParameters) {
return writeErrors;
}

private static Document toErrorDocument(MongoServerError e, int index) {
public static Document toErrorDocument(MongoServerError e, int index) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

protected would be enough.
Please add the @VisibleForExternalBackends to indicate why it’s not a private method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VisibleForExternalBackends added.

@justinchuch justinchuch force-pushed the async-support-2 branch 2 times, most recently from 578b71c to 129c171 Compare July 27, 2020 20:24
log unknown error
unwrap CompletionException
enhance error handling
Change method visibility to public for extensibility
Add tests to cover synchronous and asynchronous method call
Change method visibility to public for extensibility
Update MongoCollection
Change method visibility to public for extensibility
Change method visibility to public for extensibility
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants