Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

Fix/ub 1550 fix provisioner error message #259

Merged
merged 26 commits into from
Nov 27, 2018

Conversation

olgashtivelman
Copy link
Collaborator

@olgashtivelman olgashtivelman commented Nov 26, 2018

change error: "panic: Error starting ubiquity client" to "panic: Error starting ubiquity provisioner"


This change is Reviewable

Copy link
Contributor

@shay-berman shay-berman left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor

@shay-berman shay-berman left a comment

Choose a reason for hiding this comment

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

:lgtm:

(please make sure the text is ok with Dima)

Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on @shay-berman and @olgashtivelman)


volume/provision.go, line 102 at r2 (raw file):

		if isTimeOutError(err) {
			// The log is here to advise the user on where to look for further information
			logger.Printf("Failed to start ubiqutiy-k8s-provisioner due to connectivity issue to ubiqutiy pod")

please check text with Dima (btw i think you missed dot at the end of the sentence.)


volume/provision.go, line 109 at r2 (raw file):

}

func isTimeOutError(err error) bool {

if its easy to add 1 UT for this function go for it, but if not its ok since its very basic.

Copy link
Collaborator Author

@olgashtivelman olgashtivelman left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on @shay-berman and @olgashtivelman)


volume/provision.go, line 109 at r2 (raw file):

Previously, shay-berman wrote…

if its easy to add 1 UT for this function go for it, but if not its ok since its very basic.

it is not easy unfortunately (I will need to re-create the error) . so i skipped it since its also just for the log message and no actual logic.

Copy link
Collaborator Author

@olgashtivelman olgashtivelman left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on @shay-berman and @olgashtivelman)


volume/provision.go, line 102 at r2 (raw file):

Previously, shay-berman wrote…

please check text with Dima (btw i think you missed dot at the end of the sentence.)

Done.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants