Skip to content

Conversation

@jmikola
Copy link

@jmikola jmikola commented Oct 23, 2025

Q A
Branch? 3.x
Bug fix? no
New feature? yes
Deprecations? yes
Issues PHPORM-398
License MIT

Define a new "mongodb" handler type. It accepts an "id" reference like the old "mongo" type; however, "uri" instead of a single "host" and "port". The "uri" option is more flexible.

Additionally, the "username" and "password" options have been renamed and are no longer used to modify the connection string directly ("mongo" never applied URL encoding). Instead, the options are set in the URI options array, which does not require encoding. The "mongodb" never requires a password, as a username alone is valid for some auth mechanisms.

Lastly, a "monolog-bundle" app name is specified when the bundle constructs a MongoDB\Client instance for both "mongo" and "mongodb" handler syntax.

@jmikola jmikola force-pushed the 3.x-mongodb-handler-syntax branch from dcfe73f to a073d14 Compare October 23, 2025 02:55
@jmikola
Copy link
Author

jmikola commented Oct 23, 2025

The Fabbot failure seems unrelated:

Error: Test case methods should not have a return type. Please apply the patch below.

None of the new test methods introduced in this PR specify a return type.

@GromNaN did this not come up in your previous PR (#485)?


if (empty($handler['formatter'])) {
$formatter = new Definition('Monolog\Formatter\MongoDBFormatter');
$formatter->setPublic(false);
Copy link
Member

Choose a reason for hiding this comment

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

It's an inline service, it can't be public, you can remove this statement.

Copy link
Author

Choose a reason for hiding this comment

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

I saw that Definition has private bool $public = false. I assume most of these setPublic(false) calls throughout MonologExtension are just obsolete.

@GromNaN
Copy link
Member

GromNaN commented Oct 23, 2025

The Fabbot failure seems unrelated:

Error: Test case methods should not have a return type. Please apply the patch below.

None of the new test methods introduced in this PR specify a return type.

@GromNaN did this not come up in your previous PR (#485)?

We added the rule after. We should probably remove the return type in this branch. It's annoying to ignore fabbot failures.

Edit: you can rebase.

Define a new "mongodb" handler type. It accepts an "id" reference like the old "mongo" type; however, "uri" instead of a single "host" and "port". The "uri" option is more flexible.

Additionally, the "username" and "password" options have been renamed and are no longer used to modify the connection string directly ("mongo" never applied URL encoding). Instead, the options are set in the URI options array, which does not require encoding. The "mongodb" never requires a password, as a username alone is valid for some auth mechanisms.

Lastly, a "monolog-bundle" app name is specified when the bundle constructs a MongoDB\Client instance for both "mongo" and "mongodb" handler syntax.
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

A few ideas if you want to refine it, but otherwise it looks good to me!


$client = new Definition('MongoDB\Client', [
$handler['mongodb']['uri'],
$uriOptions,
Copy link
Member

Choose a reason for hiding this comment

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

Should we have writeConcern: 0 or 1 by default for logs?
You can accept driver options.

Copy link
Author

Choose a reason for hiding this comment

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

That can be specified in the uri option. I only wanted to break out username and password since (a) they might come from separate env vars and (b) they were supported in the original mongo syntax and I wanted to avoid dealing with URI encoding.

We can revisit when adding more options across the board.

You can accept driver options

Were you actually referring to $driverOptions (i.e. third ctor param) or just URI options?

@GromNaN
Copy link
Member

GromNaN commented Oct 23, 2025

For the error in the CI, we can add extension: mongodb to setup-php.

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.

2 participants