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.

Sign up for GitHub

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

Jump to bottom

Remove Oracle DB support. #895

Merged
merged 3 commits into from
Apr 24, 2021
Merged

Remove Oracle DB support. #895

merged 3 commits into from
Apr 24, 2021

Conversation

larsbijl
Copy link
Contributor

Remove Oracle as we no longer plan to support it.

@larsbijl
Copy link
Contributor Author

@bcipriano I've had an initial pass at removing this. please feel free to use this branch it if there is a better way of removing it.

DiegoTavares
DiegoTavares reviewed Feb 3, 2021
View reviewed changes
Copy link
Collaborator

@DiegoTavares DiegoTavares left a comment

Choose a reason for hiding this comment

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

Looks good to me, except that the tests seem to have failed.

@larsbijl
Copy link
Contributor Author

larsbijl commented Feb 6, 2021

Maybe someone with more java experience knows how to resolve this? @bcipriano @splhack ?

org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'cueDataSource' defined in class path resource [conf/spring/applicationContext-postgres-datasource.xml]: Cannot create inner bean 'util:property-path#6eb3da71' of type [org.springframework.beans.factory.config.PropertyPathFactoryBean] while setting bean property 'url'; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'util:property-path#6eb3da71': FactoryBean threw exception on object creation; nested exception is org.springframework.beans.InvalidPropertyException: Invalid property 'url' of bean class [com.imageworks.spcue.test.TestDatabaseSetupPostgres]: Getter for property 'url' threw exception; nested exception is java.lang.reflect.InvocationTargetException```

@larsbijl larsbijl changed the title Remove Oracle DB support. WIP: Remove Oracle DB support. Feb 6, 2021
@splhack
Copy link
Contributor

splhack commented Feb 6, 2021

Still some settings and code have references to Oracle.
Also I think we need to remove Trackit references too, because it seems like relies on Oracle.
Here is the patch to remove those. ./gradlew test passed on macOS.
https://gist.github.com/splhack/a0fff658d1051d77431ad2b8f58e7e17

@larsbijl
Copy link
Contributor Author

larsbijl commented Feb 7, 2021

superstar! thanks @splhack

@larsbijl larsbijl force-pushed the 8400-remove-oracle-db branch 2 times, most recently from 285134b to 64e4c9b Compare February 7, 2021 12:48
@larsbijl larsbijl mentioned this pull request Feb 7, 2021
Closed
@larsbijl larsbijl linked an issue Feb 7, 2021 that may be closed by this pull request
Drop Oracle DB support #840
Closed
bcipriano
bcipriano reviewed Feb 15, 2021
View reviewed changes
Copy link
Collaborator

@bcipriano bcipriano left a comment

Choose a reason for hiding this comment

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

I believe we can also remove the cuebot/tns/ directory, this is storing some Oracle config.

I'm seeing some unrelated changes in the diff here, possible this just needs a merge from master?

cuebot/src/main/java/com/imageworks/spcue/service/AdminManagerService.java Outdated Show resolved Hide resolved
cuebot/src/main/resources/conf/ddl/postgres/migrations/V9__Change_virt_type.sql Outdated Show resolved Hide resolved
@larsbijl
Copy link
Contributor Author

Rebased this properly this time (i hope)

@larsbijl larsbijl changed the title WIP: Remove Oracle DB support. Remove Oracle DB support. Mar 6, 2021
@splhack splhack mentioned this pull request Mar 29, 2021
Closed
bcipriano
bcipriano approved these changes Apr 11, 2021
View reviewed changes
Copy link
Collaborator

@bcipriano bcipriano left a comment

Choose a reason for hiding this comment

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

LGTM. Sorry for taking so long. Will just need one more rebase then we can merge.

@larsbijl larsbijl merged commit 49bdf56 into AcademySoftwareFoundation:master Apr 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DiegoTavares DiegoTavares DiegoTavares left review comments

@bcipriano bcipriano bcipriano approved these changes

@gregdenton gregdenton Awaiting requested review from gregdenton

@IdrisMiles IdrisMiles Awaiting requested review from IdrisMiles

@jrray jrray Awaiting requested review from jrray

@smith1511 smith1511 Awaiting requested review from smith1511

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Drop Oracle DB support
4 participants
@larsbijl @splhack @DiegoTavares @bcipriano