Skip to content

GPII-2160: Fix and test case for failure to continue with journal restore actions if one results in a failure#497

Merged
simonbates merged 3 commits intoGPII:masterfrom
amb26:GPII-2160
Feb 3, 2017
Merged

GPII-2160: Fix and test case for failure to continue with journal restore actions if one results in a failure#497
simonbates merged 3 commits intoGPII:masterfrom
amb26:GPII-2160

Conversation

@amb26
Copy link
Member

@amb26 amb26 commented Jan 22, 2017

No description provided.

@amb26
Copy link
Member Author

amb26 commented Jan 22, 2017

ok to test

@gpii-bot
Copy link

CI job passed.

@simonbates simonbates self-assigned this Jan 30, 2017
}
};

/** In the case we are servicing a "restore" rootAction, wrap "dangerous" promises which perform system actions so that
Copy link
Contributor

Choose a reason for hiding this comment

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

The long lines in this diff make it inconvenient to read using the GitHub "Unified" diff view. Could you wrap the lines please? Do we have any policy in GPII on max line length?

Looks like the GitHub Unified diff window is max 120 chars wide plus scrollbar. But the scrollbar is off screen if the diff is long. The "Split" diff mode wraps (rather than scrollbar) and I will use that for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

On my system I found that the view showed 131 characters (plus one for the + in the first column) so I've tried wrapping to that, let me know if this seems readable

/** Upgrades a promise rejection payload (or Error) by suffixing an additional "while" reason into its "message" field
* @param originError {Object|Error} A rejection payload. This should (at least) have the member `isError: true` set, as well as a String `message` holding a rejection reason.
* @param whileMsg {String} A message describing the activity which led to this error
* @return {Object} The rejected payload formed by shallow cloning the supplied argument (if it is not an `Error`) and suffixing its `message` member
Copy link
Contributor

@simonbates simonbates Jan 31, 2017

Choose a reason for hiding this comment

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

Looking at the code, I understand what this return description means. But before reading the code, I had a question: what happens if it is an Error? Maybe could rework the wording or sentence structure to make it clear that the message is always updated, either in-place (Error), or on a copy (Object).

Copy link
Member Author

Choose a reason for hiding this comment

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

Message updated

/** Transform a promise into one that always resolves, by intercepting its reject action and converting it to a logging action plus a resolve with an optionally supplied value.
* The error payload's message will be logged to `fluid.log` with the priority `fluid.logLevel.WARN`.
* @param promise {Promise} The promise to be transformed
* @param whileMsg {String} A suffix to be applied to the message, by the action of the utility `kettle.upgradeError`. This will typically begin with the text " while"
Copy link
Contributor

Choose a reason for hiding this comment

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

"kettle.upgradeError" should be "gpii.upgradeError"

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the fix

@gpii-bot
Copy link

gpii-bot commented Feb 1, 2017

CI job passed.

1 similar comment
@gpii-bot
Copy link

gpii-bot commented Feb 1, 2017

CI job passed.

@simonbates simonbates merged commit cc77cf9 into GPII:master Feb 3, 2017
@simonbates
Copy link
Contributor

Merged at b750909

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

Comments