Skip to content

Add Oracle-specific workaround for changing a column type#308

Closed
VicDeo wants to merge 1 commit into
masterfrom
fix-core-16812
Closed

Add Oracle-specific workaround for changing a column type#308
VicDeo wants to merge 1 commit into
masterfrom
fix-core-16812

Conversation

@VicDeo

@VicDeo VicDeo commented Jun 23, 2015

Copy link
Copy Markdown
Contributor

Ref owncloud/core#16812

"6.0.4 to 7.0.5 upgrade with Oracle as backend database"

Comment thread appinfo/preupdate.php Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

\OC::$server->getDatabaseConnection()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@nickvergessen it should reach stable7 ;)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well then change it in the patch that is for stable7, but on master we are not going start using private and deprecated APIs

@nickvergessen

Copy link
Copy Markdown
Contributor

Am I right, that this would need backporting all the way to stable7?

@VicDeo

VicDeo commented Jun 23, 2015

Copy link
Copy Markdown
Contributor Author

@nickvergessen I think so
cc @DeepDiver1975

Comment thread appinfo/preupdate.php Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

\OC::$server->getLogger()->warn()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

\OCP\Util::writeLog is not deprecated in master

@DeepDiver1975

Copy link
Copy Markdown
Member

Am I right, that this would need backporting all the way to stable7?

we have to as we need to fix migration on oracle for oc6 -> oc7

@nickvergessen

Copy link
Copy Markdown
Contributor

Okay, I will try to take this over from here on then

@nickvergessen

Copy link
Copy Markdown
Contributor

Okay, I will try to take this over from here on then

Anyway, feel free to continue @VicDeo

@DeepDiver1975

Copy link
Copy Markdown
Member

Anyway, feel free to continue @VicDeo

yes please @VicDeo - your task 🙊

Comment thread appinfo/preupdate.php Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should be a comparison "if lower than", not equals... Otherwise 1.1.1 will not run this update, but it should

@nickvergessen

Copy link
Copy Markdown
Contributor

Actually we don't need this file in stable8 and master?
Updating from 6 to 8 is not allowed?

@VicDeo
VicDeo force-pushed the fix-core-16812 branch 2 times, most recently from c9e4d3d to cce827a Compare June 24, 2015 18:17
@VicDeo

VicDeo commented Jun 24, 2015

Copy link
Copy Markdown
Contributor Author

@nickvergessen updated in accordance with your comments

@VicDeo

VicDeo commented Jun 24, 2015

Copy link
Copy Markdown
Contributor Author

@nickvergessen

Updating from 6 to 8 is not allowed?

Right.
But disabling Activity, updating 6 -> 7 -> 8 -> xxx and enabling Activity is allowed.

@DeepDiver1975

Copy link
Copy Markdown
Member

But disabling Activity, updating 6 -> 7 -> 8 -> xxx and enabling Activity is allowed.

ouch - so true ... do we want to support such a scenario?

@karlitschek

@nickvergessen

Copy link
Copy Markdown
Contributor

Well okay, the let's say the activity app does not support a major version jump:
1.1.x > 1.2.x > 2.0.x
So that should work as well?

@DeepDiver1975

Copy link
Copy Markdown
Member

Well okay, the let's say the activity app does not support a major version jump:
1.1.x > 1.2.x > 2.0.x
So that should work as well?

well - the main problem here is that app upgrade are not triggered if they are not active - which is okay.
But this can result in the behavior as explained by @VicDeo in #308 (comment)

Until we find a better mechanism to handle migrations more seemless (pssst owncloud/core#14851 🙊 ) we might simply add the migration to every release branch - even if they are never executed?

@nickvergessen

Copy link
Copy Markdown
Contributor

So we also have to bring back the update.php which was dropped because of the normally unsupported update-way?
24ab014

I mean if someone encounters the problem, we can still say, hey please upload the files of 1.2.x (oC 8.0) first then update to that version and then update to 2.0 (oC 8.1)?

@DeepDiver1975

Copy link
Copy Markdown
Member

So we also have to bring back the update.php which was dropped because of the normally unsupported update-way?
24ab014

I mean if someone encounters the problem, we can still say, hey please upload the files of 1.2.x (oC 8.0) first then update to that version and then update to 2.0 (oC 8.1)?

valid point as well ....

@VicDeo

VicDeo commented Jun 26, 2015

Copy link
Copy Markdown
Contributor Author

I never argue for a long time (and don't use Oracle anyway).
Let those unsupported migrators die if it looks correct from your standpoint.

@nickvergessen

Copy link
Copy Markdown
Contributor

Yeah, lets just fix this in 6>7 then

@nickvergessen
nickvergessen deleted the fix-core-16812 branch June 27, 2015 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants