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.

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

fix data migration issues when migrating from really old databases #507

Closed
wants to merge 1 commit into from

Conversation

WattsInABox
Copy link

We had an issue when migrating our 8-month old database. Apparently, there were some revisions that had reviewers that were missing the src_type (or type) in their data column. These two guard statements fixed that issue for us.

@epriestley
Copy link
Member

Which migration did this fix, specifically? I'd rather fix the migration than adjust the EdgeEditor, if possible. Do you have the original error/trace?

@WattsInABox
Copy link
Author

I would, too, but I'm not a PHP guy anymore so it was a little difficult finding the fix. 20131004.dxreviewers.php. I didn't save off the trace, but it was on line 192 of PhabricatorEdgeConfig.php:

if (!$class) {
      throw new Exception(
        "Edges are not available for objects of type '{$phid_type}'!");
    }

and $phid_type ended up being equal to '????'

@epriestley
Copy link
Member

Thanks! I'll see if I can come up with a more surgical fix. We're in the middle of some other stuff so it might take me a couple of days to get to, but you're not blocked on this anymore, right? I can look at it sooner if you're still having issues, but it sounds like things worked after your patch?

@WattsInABox
Copy link
Author

Oh yeah, we're good. I just wanted to contribute it back and make you aware so other people don't have the same issue.

@epriestley
Copy link
Member

@epriestley epriestley closed this Apr 22, 2014
epriestley pushed a commit that referenced this pull request Apr 23, 2014
Summary: See #507

Test Plan: This is hard to test since the migration no longer runs against HEAD, but pull 507 strongly implies this is the correct fix.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8837
@WattsInABox
Copy link
Author

thanks, btw! I know this was forever ago.

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.

2 participants