Adding context to database provider

[mfreeman451]

Pull Request Template

Description:

• Added the ability to pass a context down through the DB provider to the Connect() call
• Connect() now returns an error.
• Currently the Connect() interface doesn't accept a context, invalid connections to databases will not be known.

Breaking Changes (if applicable):

• Connect() calls in the DB provider will now return an error and require a context to be passed.
• This breaks the ability to daisy chain method calls.

Additional Information:

• Mention any relevant dependencies or external libraries used.
• Include screenshots or code snippets (if necessary) to clarify the changes.
• Closes mongo driver doesnt support contexts #1117

Checklist:

• I have formatted my code using goimport and golangci-lint.
• All new code is covered by unit tests.
• This PR does not decrease the overall code coverage.
• I have reviewed the code comments and documentation for clarity.

Thank you for your contribution!

[vipul-rawat]

Hey @mfreeman451, the mocks have not been updated, but before that can we discuss some approaches and implementations?

I feel like AddMongo, AddClickhouse, etc... should not return any error but log the error, as we do with the SQL, Redis, etc...

One other train of thought would be to FATAL log the errors, as it would panic the code even if it starts as the handler might expect the databases to be connected.

[aryanmehrotra]

@mfreeman451 can you fix the reveiw comments?
We would not want to keep a PR open for more than 2 weeks - for better maintainability.

[ccoVeille]

See #1118 (comment)

[mfreeman451]

cleaned up and updated

[coolwednesday]

Hey @mfreeman451,

I was going through the code and I believe the current implementation might introduce a breaking change. To prevent that, here's what I suggest:

• Let's focus solely on MongoDB for now. We should avoid making changes in other datasources for now. Please revert those changes.

• We can create an interface that embeds the existing provider interface, say providerWithContext and introduces an additional method, ConnectWithContext(ctx context.Context). This can help us to handle, only MongoDB-specific logic without affecting other providers in this PR.

• Replace the current provider implementation with this new interface for MongoProvider, enabling the optional use of the ConnectWithContext method.

• In externaldb.go, where we are injecting the MongoDB provider using AddMongo, we will replace the call to Connect() with ConnectWithContext(ctx context.Context).

• To handle context passing efficiently, we can use the functional options pattern. This way, context can be optionally passed to the connection method without breaking the existing flow.

I believe the approach I suggested is one way to address this. However, if you have a better or more efficient solution, feel free to go ahead and use it to resolve the issue. I’m open to exploring alternatives that might work better for the use case.

Let me know your thoughts on this!

[Umang01-hash]

@mfreeman451 are you proceeding towards the review-changes suggested by @coolwednesday ?

[aryanmehrotra]

Please reopen once the changes are fixed and comments are resolved as it has been more than 2 weeks of last activity.