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

Allow customizing URI options more #126

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

scibiliad
Copy link

This PR resolve the issue #125

@scibiliad scibiliad force-pushed the allow_customizing_URI_options_more branch from 63ddbda to f9dfc2a Compare May 10, 2023 16:13
@ilario-pierbattista ilario-pierbattista self-requested a review May 11, 2023 05:28
@scibiliad scibiliad force-pushed the allow_customizing_URI_options_more branch from f9dfc2a to d15bc89 Compare June 1, 2023 15:23
docs/Documentation.md Show resolved Hide resolved
docs/Documentation.md Outdated Show resolved Hide resolved
docs/Documentation.md Outdated Show resolved Hide resolved
src/Services/ClientRegistry.php Outdated Show resolved Hide resolved
src/Services/ClientRegistry.php Outdated Show resolved Hide resolved
tests/Unit/Services/ClientRegistryTest.php Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
@bruno-buiret
Copy link
Contributor

Hi, sorry, but do you have time to finish the pull request? Thank you so much

@scibiliad
Copy link
Author

Hi, I'll try to complete it in the next few weeks

@bruno-buiret
Copy link
Contributor

Thank you so much!

@scibiliad scibiliad force-pushed the allow_customizing_URI_options_more branch 2 times, most recently from 3d9ef15 to fe1a975 Compare October 19, 2024 14:01
@scibiliad scibiliad marked this pull request as ready for review October 19, 2024 14:05
@ilario-pierbattista ilario-pierbattista force-pushed the allow_customizing_URI_options_more branch from fe1a975 to c7a9996 Compare December 7, 2024 07:22
@ilario-pierbattista
Copy link
Member

Hey @bruno-buiret, thank you for opening the issue, we're almost there.

Before merging this, would you give use some feedback? Does this satisfy all your needs?

You can install it with:

composer require facile-it/mongodb-bundle:dev-allow_customizing_URI_options_more

The branch is rebased on 1.6.2 so that everything is up to date.

Thank you in advance

@bruno-buiret
Copy link
Contributor

@ilario-pierbattista, so sorry, work has been kind of hectic.

I just created a basic Symfony 7.2 application with PHP 8.4 and added the bundle with the Composer command you provided. I was able to customize the appname parameter and it showed in $currentOp, so thanks!

A few questions if I may,

  1. Would it be possible to have the client's name as a second parameter so that logic can be different according to the client being built?
interface UriOptionsInterface
{
    /**
     * ...
     */
    public function buildUriOptions(array $clientConfiguration, string $clientName): array;
}
  1. Is it normal that for DriverOptionsInterface the full $conf is used as a parameter instead of only $conf['driverOptions'] in \Facile\MongoDbBundle\Services\ClientRegistry::buildClientConfiguration()?
        $conf['driverOptions'] = [];
        if ($this->driverOptionsService instanceof DriverOptionsInterface) {
            $conf['driverOptions'] = $this->driverOptionsService->buildDriverOptions($conf);
        }
  1. If it's possible to add the client's name for UriOptionsInterface, it would be quite cool to have it for DriverOptionsInterface too ^^

Thank you so much for this and I hope this help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants