Refract/connection #46
Merged
Conversation
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
- Added `validateAuthType` method to verify auth types. - Enhanced `setOptions` to use null coalescing for index prefix. - Simplified `setIndex` method for cleaner logic. - Replaced multiple `str_contains` checks with single line logic. What did the code say to the bug? You're no match for my refactor skills! 🐛🔥
…ation - Added `_sanitizeConfig` to merge config with default settings. - Implemented `_validateConnection` to verify required config parameters based on `auth_type`. - Combined HTTP and Cloud connection setups into `buildConnection` method. - Updated visibility and type hints for class properties and methods. - Introduced new tests for connection handling, disconnect, and validation scenarios. Making connections so smooth, Arnold would be proud! 💪
- Introduced `getClientInfo` method to retrieve Elasticsearch client information. - Added `AuthenticationException` for better error handling during connection build. - Configured connection name dynamically instead of hardcoding. - Enhanced docstrings for connection validation and builder options. - Implemented new tests for prefix handling and client info retrieval. - Refactored existing tests to use `expect` instead of `$this->assertInstanceOf`. Who needs a crystal ball when you have comprehensive client info? 🔮
- Introduced `getClientInfo` method to retrieve Elasticsearch client information. - Added `AuthenticationException` for better error handling during connection build. - Configured connection name dynamically instead of hardcoding. - Enhanced docstrings for connection validation and builder options. - Implemented new tests for prefix handling and client info retrieval. - Refactored existing tests to use `expect` instead of `$this->assertInstanceOf`. Who needs a crystal ball when you have comprehensive client info? 🔮
- Added 'logging' option to Elasticsearch connection config. - Enabled conditional logging setup in '_builderOptions' based on config. - Ensured logging can be toggled via configuration settings. Because logging should be more than just a silent observer now! 📜
|
Looks good so far. Will merge on next release. Thanks |
|
Great work @use-the-fork |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR aims to refractor the current connection. It removes the dependencies on using config as that causes collisions when using multiple elastic connections. Instead, it uses the config that's passed to the connection by elastic.
I also added sanitization and validation of the connection to guarantee all connection values are set when creating the connection.
Finally, it adds a new option "logging" which enables the built-in elastic search logger but passes in and uses the current Laravel log instance based on the user's stack.
https://www.elastic.co/guide/en/elasticsearch/client/php-api/current/enabling_logger.html