-
Notifications
You must be signed in to change notification settings - Fork 95
Drop the auth-DB remove in test-esx. #877
Conversation
There was a problem hiding this 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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this 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) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
Fix a small bug in function "get_datastore_url" and "get_datastore_name" to handle the _DEFAULT_DS correctly.
This PR includes two changes:
test-esx passed. Please review this change.