[stable9.1] Don't rerepair unmerged shares if updating from OC > 9.0.3 #27176

Merged
merged 1 commit into from Feb 20, 2017

Conversation

Projects
None yet
2 participants
@PVince81
Member

PVince81 commented Feb 17, 2017

Because unmerged shares existed only between OC 9.0.0 and 9.0.3 included and in OC <= 9.0.0, when someone updated to OC > 9.0.3 they already had the repair routine running.

Tested as follows:

  1. Set breakpoint on the changed line
  2. Set version to 9.0.3 in config.php
  3. Run occ upgrade
  4. See that code runs into the repair part
  5. Set version to 9.0.4 in config.php
  6. Run occ upgrade
  7. See that code runs skips the repair part

Please review @VicDeo @jvillafanez @IljaN

@PVince81 PVince81 added this to the 9.1.5 milestone Feb 17, 2017

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Feb 17, 2017

Member
  • forward port to master: on master, only run the repair if updating from OC 9.0.0
Member

PVince81 commented Feb 17, 2017

  • forward port to master: on master, only run the repair if updating from OC 9.0.0
@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Feb 17, 2017

Member

Seems I broke the case where updating from OC 9.0.0...

Member

PVince81 commented Feb 17, 2017

Seems I broke the case where updating from OC 9.0.0...

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Feb 17, 2017

Member

Fixed now and added unit test to enforce the version check.

Member

PVince81 commented Feb 17, 2017

Fixed now and added unit test to enforce the version check.

@jvillafanez

This comment has been minimized.

Show comment
Hide comment
@jvillafanez

jvillafanez Feb 20, 2017

Member

Any reason why it's unsafe to rerun the repair step? Is it possible that unmerged shares will appear in the future?

The code looks fine, but I'm wondering if we're just putting a band-aid

Member

jvillafanez commented Feb 20, 2017

Any reason why it's unsafe to rerun the repair step? Is it possible that unmerged shares will appear in the future?

The code looks fine, but I'm wondering if we're just putting a band-aid

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Feb 20, 2017

Member

It's not unsafe but it wastes time for huge setups as it can take many hours just for nothing.

Unmerged shares shouldn't reappear as they were a result of a bug (mostly missing logic that wasn't ported from the old sharing code to the new sharing code)

Member

PVince81 commented Feb 20, 2017

It's not unsafe but it wastes time for huge setups as it can take many hours just for nothing.

Unmerged shares shouldn't reappear as they were a result of a bug (mostly missing logic that wasn't ported from the old sharing code to the new sharing code)

@jvillafanez

This comment has been minimized.

Show comment
Hide comment
@jvillafanez

jvillafanez Feb 20, 2017

Member

Ok, make sense. 👍

Member

jvillafanez commented Feb 20, 2017

Ok, make sense. 👍

@PVince81 PVince81 merged commit a2bcf97 into stable9.1 Feb 20, 2017

3 checks passed

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 PVince81 deleted the stable9.1-skiprepairunmergedshareswhennotneeded branch Feb 20, 2017

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