Skip to content
This repository has been archived by the owner on Nov 9, 2020. It is now read-only.

Drop the auth-DB remove in test-esx. #877

Merged
merged 4 commits into from
Jan 27, 2017
Merged

Drop the auth-DB remove in test-esx. #877

merged 4 commits into from
Jan 27, 2017

Conversation

lipingxue
Copy link
Contributor

@lipingxue lipingxue commented Jan 24, 2017

This PR includes two changes:

  1. drop the auth-DB remove in test-esx (fixes issue Drop DB deletion from test-esx #875)
  2. Fix a small bug in function "get_datastore_url" and "get_datastore_name" to handle the _DEFAULT_DS correctly.

test-esx passed. Please review this change.

Copy link
Contributor

@shuklanirdesh82 shuklanirdesh82 left a comment

Choose a reason for hiding this comment

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

test-esx changes looks good to me.

@lipingxue do we have issue for default_datastore url if so please link it here?

Copy link
Contributor

@shaominchen shaominchen left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

@msterin msterin left a comment

Choose a reason for hiding this comment

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

overall looks good - a nit and a question inside

@@ -210,6 +210,7 @@ def get_default_datastore(self, conn):
return str(error_info), None
else:
datastore_url = result[auth_data_const.COL_DEFAULT_DATASTORE_URL]
logging.debug("get_default_datastore: datastore_url=%s", datastore_url)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit - why this is dropped ? Looks like a useful debug info for me.
It's a nit, feel free to merge with it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is NOT dropped. I added this logging in my change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is NOT dropped, I added this log in my change :)

Copy link
Contributor

Choose a reason for hiding this comment

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

oops. you are right :-) !

unit_test_array=($TEST_URL_ARRAY)
numServers=${#unit_test_array[@]}
DRONE_BUILD_NUMBER=${DRONE_BUILD_NUMBER:=0}
prevBuildStatus=`drone build info vmware/docker-volume-vsphere $(( $DRONE_BUILD_NUMBER-$numServers ))`
outArray=($prevBuildStatus)
total_builds=`drone build list vmware/docker-volume-vsphere| wc -l`

govc datastore.mkdir docker-volume-vsphere/$DRONE_BUILD_NUMBER
govc datastore.mkdir $DS docker-volume-vsphere/$DRONE_BUILD_NUMBER
Copy link
Contributor

Choose a reason for hiding this comment

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

so on which DS it was doing mkdir before the change ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is not from me. It was made by Ritesh in master, I rebase my change with master and then you see the change. I have removed from my commit log.

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

Successfully merging this pull request may close these issues.

None yet

5 participants