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

PHPC-495: Upgrade bundled libbson and libmongoc to 1.3.0 #141

Merged
merged 22 commits into from
Dec 2, 2015

Conversation

derickr
Copy link
Contributor

@derickr derickr commented Nov 20, 2015

(Also includes fix for PHPC-488)

phongo_server_init(return_value, intern->client, server_id TSRMLS_CC);
selected_server = mongoc_topology_select(intern->client->topology, MONGOC_SS_READ, readPreference, MONGOC_SS_DEFAULT_LOCAL_THRESHOLD_MS, &error);
if (selected_server) {
phongo_server_init(return_value, intern->client, selected_server->id TSRMLS_CC);
Copy link
Contributor

Choose a reason for hiding this comment

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

My bad. This leaks the server_description.

mongoc_server_description_destroy (selected_server);
Is needed after the server_init ()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@derickr derickr force-pushed the PHPC-495-upgrade-to-libmongoc-1.3.0 branch 2 times, most recently from 5adc83c to bc05644 Compare December 2, 2015 17:04
} else if (!sd1) {
phongo_throw_exception(PHONGO_ERROR_RUNTIME TSRMLS_CC, "%s", error1.message);
} else {
phongo_throw_exception(PHONGO_ERROR_RUNTIME TSRMLS_CC, "%s", error2.message);
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be good to use the same format we do above (when accessing descriptions files). We can use:

  • "Failed to get server descriptions: %s and %s"
  • "Failed to get server description: %s"

This provides an easily searchable string for the error and avoids any edge case where the message may be empty (if libmongoc misbehaves).

@bjori
Copy link
Contributor

bjori commented Dec 2, 2015

I'd merge #146 to resolve that that Werror issue in mongoc. There is no reason why phongo does this.
phongo should care about its own warnings, not mongoc.

Do feel free to file a ticket for it in mongoc, but it definitely should not halt phongo development like this.

@derickr derickr force-pushed the PHPC-495-upgrade-to-libmongoc-1.3.0 branch from caa4250 to bc9ebcc Compare December 2, 2015 18:12
@derickr derickr merged commit bc9ebcc into mongodb:master Dec 2, 2015
derickr added a commit that referenced this pull request Dec 2, 2015
derickr added a commit to mongodb/mongo-hhvm-driver that referenced this pull request Dec 11, 2015
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