feat: Oracle db migration (#2015) #2197
Conversation
Samar1110
changed the title
There was a problem hiding this comment.
@Samar1110 Here are a few changes for your PR:
- Remove all the newly added files inside
datasource/filedirectory (hello.txt,temp.csv,temp.json,temp.txt) as they are not related to scope of this PR and feels like uploaded by mistake. - The changes in file
go.work.sumshould also be reverted.
| Select(ctx context.Context, dest any, query string, args ...any) error | ||
| Exec(ctx context.Context, query string, args ...any) error | ||
|
|
||
| HealthCheck(ctx context.Context) (any, error) |
There was a problem hiding this comment.
Why is this method needed for migrations?
There was a problem hiding this comment.
Added the HealthCheck(ctx context.Context) (any, error) method to the Oracle interface in the migration package to align it with the OracleDB interface defined in the container package. Without this method, the *MockOracle generated for testing cannot be used as a container.OracleDB value, causing the compiler error:
"cannot use mockOracle (variable of type *MockOracle) as container.OracleDB value in assignment: *MockOracle does not implement container.OracleDB (missing method HealthCheck)".
As I am using
mockContainer.Oracle = mockOraclein some functions, it takes the reference from pkg/gofr/container/datasource.go, which requires the HealthCheck method for compatibility. To resolve this, I have added the HealthCheck method in pkg/gofr/migration/interface.go.
By adding the HealthCheck method, the mock satisfies the interface requirements, ensuring type compatibility and eliminating this assignment error.
There was a problem hiding this comment.
@Samar1110 I agree but if you notice many of the interfaces like SQL, Redis, Mongo, ArrangoDB etc all have HealthCheck() method not implemented yet they align with their interface definition in container.
You can see how their test have been written or implemented.
| Run(migrationMap, mockContainer) | ||
| } | ||
|
|
||
| func TestOracleMigration_FailCreateMigrationTable(t *testing.T) { |
There was a problem hiding this comment.
The tests for Oracle Migration should be placed inside oracle_test.go not here.
There was a problem hiding this comment.
Do I need to shift only this test -> TestOracleMigration_FailCreateMigrationTable or all test related to Oracle migration in this file to oracle_test.go ?
There was a problem hiding this comment.
You should place all test related to Oracle migration in this file to oracle_test.go
| } | ||
|
|
||
| const ( | ||
| CheckAndCreateOracleMigrationTable = ` |
There was a problem hiding this comment.
Why is this constant exported? Can we please unexport it if not needed outside of the package.
There was a problem hiding this comment.
Sure I would do that
| @@ -207,3 +208,130 @@ func initializeClickHouseRunMocks(t *testing.T) (*MockClickhouse, *container.Con | |||
|
|
|||
There was a problem hiding this comment.
In the method initializeClickHouseRunMocks we need to mark Oracle as nil else it's tests will fail.
There was a problem hiding this comment.
Sure , I will mark it as nil
|
@Samar1110 Please resolve the merge conflicts in your PR. You are near to completing this PR. Let's close it soon and if you need any help please let us know. |
Sure @Umang01-hash , by today I would complete the work |
Pull Request: OracleDB Migration Support Integration
Description:
Details of the Change
Motivation & Benefits:
Checklist:
goimportandgolangci-lint.