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

CSVS-54 English was fixed #81

Merged
merged 14 commits into from
Oct 20, 2017
Merged

CSVS-54 English was fixed #81

merged 14 commits into from
Oct 20, 2017

Conversation

MedvedevBW
Copy link
Collaborator

No description provided.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 82.643% when pulling 249fd18 on CSVS-54 into fbabd40 on develop.

placccebo
placccebo previously approved these changes Oct 17, 2017
@@ -30,18 +30,18 @@ import org.slf4j.LoggerFactory
/**
* Class is responsible for interaction with CloudStack server with help of CloudStackTaskCreator
*
* @param cloudStackTaskCreator allows for creating task for interaction with CloudStack
* @param settings contains the settings for interaction with CloudStack
* @param cloudStackTaskCreator enables creation tasks for interaction with CloudStack
Copy link
Collaborator

Choose a reason for hiding this comment

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

enables the creation of tasks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 82.643% when pulling f4eac5e on CSVS-54 into fbabd40 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 82.643% when pulling d9af398 on CSVS-54 into fbabd40 on develop.

@@ -120,20 +120,20 @@ class CloudStackService(cloudStackTaskCreator: CloudStackTaskCreator,
cloudStackTaskCreator.domainParameter -> vm.domainId.toString
))
).accountList.accounts.getOrElse(
throw new CloudStackEntityDoesNotExistException(s"The vm: $vmId does not include account with " +
throw new CloudStackEntityDoesNotExistException(s"Vm: $vmId does not include an account with " +
Copy link
Collaborator

Choose a reason for hiding this comment

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

Vm: $vmId does not include the account with "

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@@ -51,10 +51,10 @@ class CloudStackEventHandler(controller: CloudStackVaultController)
} match {
case Success(x) => x
case Failure(e: JsonParseException) =>
logger.warn("Can not to parse record json, the empty CloudStackEvent will be return")
logger.warn("Can not to parse record: \"" + s"$record" + "\", the empty CloudStackEvent is returned")
Copy link
Collaborator

Choose a reason for hiding this comment

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

logger.warn("Can not parse the record: "" + s"$record" + "", the empty CloudStackEvent will be returned")

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

CloudStackEvent(None, None, None)
case Failure(e: Throwable) =>
logger.error("Exception was thrown while record was being deserialized")
logger.error("The process of record: \"" + s"$record" + "\"" + s"deserializing thrown an exception: $e")
Copy link
Collaborator

Choose a reason for hiding this comment

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

logger.error("Deserialization process of record: "" + s"$record" + """ + s" threw an exception: $e")
or
logger.error("Exception: $e occurred during the deserialization of the record: "" + s"$record" + """ ")

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

event.status.getOrElse(Other) == CloudStackEvent.Status.Completed //Event must be handle when status of event Completed
case _ => //but first event have a signature such as {"details":"...","status":"Completed","event":"..."}
false //and don't have an entityuuid, so we must to check entityuuid.
event.status.getOrElse(Other) == CloudStackEvent.Status.Completed //Event must be handled when status of event is Completed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

move this comment to method description

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

false //and don't have an entityuuid, so we must to check entityuuid.
event.status.getOrElse(Other) == CloudStackEvent.Status.Completed //Event must be handled when status of event is Completed.
case _ => //The first event has a signature such as {"details":"...","status":"Completed","event":"..."},
false //which doesn’t contain an entityuuid field, so we must to check entityuuid.
Copy link
Collaborator

Choose a reason for hiding this comment

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

so we must check entityuuid.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@@ -93,7 +111,7 @@ class CloudStackVaultControllerTestSuite extends FlatSpec with BaseTestSuite wit
}


"handleAccountDelete" should "delete secret by default path if node with token does not exist" in {
"handleAccountDelete" should "delete secret if account node does not exist" in {
Copy link
Collaborator

Choose a reason for hiding this comment

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

"handleAccountDelete" should "delete secret if account znode does not exist"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

@@ -119,7 +137,7 @@ class CloudStackVaultControllerTestSuite extends FlatSpec with BaseTestSuite wit
assert(pathsForCheckIsExistNode == checkedIsExistNodePaths, "checkedIsExistNodePaths is wrong")
}

"handleVmDelete" should "get token from Zookeeper node, revoke it and delete secret by path in token information" in {
"handleVmDelete" should "get tokens from Zookeeper nodes, revoke them and then delete secret and policies" in {
Copy link
Collaborator

Choose a reason for hiding this comment

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

"handleVmDelete" should "get VM tokens from ZooKeeper, revoke them and then delete secret and policies" in

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

@@ -156,7 +174,7 @@ class CloudStackVaultControllerTestSuite extends FlatSpec with BaseTestSuite wit
assert(pathForCheckDeletionNode == checkedDeletionNodePaths, "checkedDeletionNodePaths is wrong")
}

"handleVmDelete" should "delete secret by default path if node with token does not exist" in {
"handleVmDelete" should "delete secret if VM node does not exist" in {
Copy link
Collaborator

Choose a reason for hiding this comment

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

znode?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes

@@ -187,7 +205,7 @@ class CloudStackVaultControllerTestSuite extends FlatSpec with BaseTestSuite wit
//* User Creation *
//* *
//*************************************
"handleUserCreate" should "creates new tokens (read, write) and put it into zooKeeper nodes and cloudStack user tags" in {
"handleUserCreate" should "create new tokens (read, write) and put them into zooKeeper nodes and cloudStack user tags" in {
Copy link
Collaborator

Choose a reason for hiding this comment

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

ZooKeeper

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

@@ -228,7 +246,7 @@ class CloudStackVaultControllerTestSuite extends FlatSpec with BaseTestSuite wit
assert(checkedNewTags.toSet == expectedVaultTagsForAccount.toSet)
}

"handleUserCreate" should "gets tokens (read, write) from zooKeeper nodes and write it into cloudStack user tags" in {
"handleUserCreate" should "get tokens (read, write) from zooKeeper nodes and put them into cloudStack user tags" in {
Copy link
Collaborator

Choose a reason for hiding this comment

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

"handleUserCreate" should "get account tokens (read, write) from ZooKeeper and put them into CloudStack user tags"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

…o CSVS-54

# Conflicts:
#	README.md
#	src/main/scala/com/bwsw/cloudstack/vault/server/cloudstack/util/CloudStackTaskCreator.scala
#	src/test/scala/com/bwsw/cloudstack/vault/server/cloudstack/util/CloudStackTaskCreatorTestSuite.scala
@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 82.683% when pulling e26d8f4 on CSVS-54 into f87904a on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 82.683% when pulling 5d64828 on CSVS-54 into f87904a on develop.

@@ -51,10 +51,10 @@ class CloudStackEventHandler(controller: CloudStackVaultController)
} match {
case Success(x) => x
case Failure(e: JsonParseException) =>
logger.warn("Can not to parse record: \"" + s"$record" + "\", the empty CloudStackEvent is returned")
logger.warn("Can not to parse the record: \"" + s"$record" + "\", the empty CloudStackEvent will be returned")
Copy link
Collaborator

Choose a reason for hiding this comment

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

there is still a mistake, see the previous review

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"to" was removed

/**
* Event must be handled when status of event is Completed.
* The first event has a signature such as {"details":"...","status":"Completed","event":"..."},
* which does not contain an entityuuid field, so we must check entityuuid.
Copy link
Collaborator

Choose a reason for hiding this comment

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

must -> have to

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is in both case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 82.683% when pulling 66e7550 on CSVS-54 into f87904a on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 82.683% when pulling df59cea on CSVS-54 into f87904a on develop.

@MedvedevBW MedvedevBW merged commit 5cb6b08 into develop Oct 20, 2017
@MedvedevBW MedvedevBW deleted the CSVS-54 branch October 20, 2017 08:53
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.

None yet

3 participants