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.

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

Expose the needupgrade status #26209

Merged
merged 1 commit into from Sep 29, 2016

Conversation

Projects
None yet
6 participants
@kprovost
Contributor

kprovost commented Sep 24, 2016

During upgrades, before the DB migration is complete, the system is not
usable, but there's no way for monitoring systems to detect this.
Add the 'needupgrade' field to the status json so monitoring systems can
detect this.

@CLAassistant

This comment has been minimized.

Show comment
Hide comment
@CLAassistant

CLAassistant Sep 24, 2016

CLA assistant check
All committers have signed the CLA.

CLAassistant commented Sep 24, 2016

CLA assistant check
All committers have signed the CLA.

@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Sep 24, 2016

@kprovost, thanks for your PR! By analyzing the annotation information on this pull request, we identified @butonic, @MorrisJobke and @DeepDiver1975 to be potential reviewers

@kprovost, thanks for your PR! By analyzing the annotation information on this pull request, we identified @butonic, @MorrisJobke and @DeepDiver1975 to be potential reviewers

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Sep 27, 2016

Member

Make sense. Not sure if it should be called "needupgrade" as someone not knowing it and seeing it might expect the value to mean "this is an old version and needs updating" while it only means "this instance's code is on the latest version but the upgrade routine needs to be run".

Should it be called "needsDbUpgrade" instead ?
I'd also prefer to not have the value visible at all instead of being false, basically only add it to the array when it's true.

CC @DeepDiver1975 @butonic

Member

PVince81 commented Sep 27, 2016

Make sense. Not sure if it should be called "needupgrade" as someone not knowing it and seeing it might expect the value to mean "this is an old version and needs updating" while it only means "this instance's code is on the latest version but the upgrade routine needs to be run".

Should it be called "needsDbUpgrade" instead ?
I'd also prefer to not have the value visible at all instead of being false, basically only add it to the array when it's true.

CC @DeepDiver1975 @butonic

@DeepDiver1975 DeepDiver1975 added this to the 9.2 milestone Sep 27, 2016

@DeepDiver1975

This comment has been minimized.

Show comment
Hide comment
@DeepDiver1975

DeepDiver1975 Sep 27, 2016

Member

I'd also prefer to not have the value visible at all instead of being false, basically only add it to the array when it's true.

👍

Member

DeepDiver1975 commented Sep 27, 2016

I'd also prefer to not have the value visible at all instead of being false, basically only add it to the array when it's true.

👍

@DeepDiver1975

This comment has been minimized.

Show comment
Hide comment
@DeepDiver1975

DeepDiver1975 Sep 27, 2016

Member

Should it be called "needsDbUpgrade" instead ?

not sure - there is no need from my pov to add too much technical details. 'needsUpgrade' is enough I guess

Member

DeepDiver1975 commented Sep 27, 2016

Should it be called "needsDbUpgrade" instead ?

not sure - there is no need from my pov to add too much technical details. 'needsUpgrade' is enough I guess

@kprovost

This comment has been minimized.

Show comment
Hide comment
@kprovost

kprovost Sep 27, 2016

Contributor

I have no objections to changing the name, but I think I'd prefer always displaying the value. That way monitoring scripts (or other users) can easily tell the difference between a version that supports the field and a version that does not.

It's JSON, so clients just end up ignoring unknown/unsupported fields anyway.

Contributor

kprovost commented Sep 27, 2016

I have no objections to changing the name, but I think I'd prefer always displaying the value. That way monitoring scripts (or other users) can easily tell the difference between a version that supports the field and a version that does not.

It's JSON, so clients just end up ignoring unknown/unsupported fields anyway.

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Sep 27, 2016

Member

Fine by me then.

Can you rename it to "needsDbUpgrade" ? Because if people see it set to false they might believe it's an outdated OC version

Member

PVince81 commented Sep 27, 2016

Fine by me then.

Can you rename it to "needsDbUpgrade" ? Because if people see it set to false they might believe it's an outdated OC version

Expose the needupgrade status
During upgrades, before the DB migration is complete, the system is not
usable, but there's no way for monitoring systems to detect this.
Add the 'needupgrade' field to the status json so monitoring systems can
detect this.
@kprovost

This comment has been minimized.

Show comment
Hide comment
@kprovost

kprovost Sep 29, 2016

Contributor

Done in cf0f297. (Comment because I don't know if the push triggered notifications on GitHub).

Contributor

kprovost commented Sep 29, 2016

Done in cf0f297. (Comment because I don't know if the push triggered notifications on GitHub).

@DeepDiver1975

This comment has been minimized.

Show comment
Hide comment
Member

DeepDiver1975 commented Sep 29, 2016

👍

@DeepDiver1975 DeepDiver1975 merged commit 181c0bb into owncloud:master Sep 29, 2016

4 of 5 checks passed

VersionEye Some dependencies have no license.
Details
Scrutinizer No new issues
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details
@PVince81

This comment has been minimized.

Show comment
Hide comment
Member

PVince81 commented Oct 4, 2016

@Emil-G

This comment has been minimized.

Show comment
Hide comment
@Emil-G

Emil-G Jul 19, 2017

With OC10 the original meaning is used, however the name is the new one.

If one clustered server does a markedplace app upgrade, the other servers goes into needsDbUpgrade mode. Perhaps misleading?

Emil-G commented Jul 19, 2017

With OC10 the original meaning is used, however the name is the new one.

If one clustered server does a markedplace app upgrade, the other servers goes into needsDbUpgrade mode. Perhaps misleading?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment