Skip to content

GEODE-5971: Refactor durable client commands to extend GfshCommand#2858

Merged
jdeppe-pivotal merged 3 commits intoapache:developfrom
jdeppe-pivotal:feature/GEODE-5971-refactor-durable-client-commands
Nov 16, 2018
Merged

GEODE-5971: Refactor durable client commands to extend GfshCommand#2858
jdeppe-pivotal merged 3 commits intoapache:developfrom
jdeppe-pivotal:feature/GEODE-5971-refactor-durable-client-commands

Conversation

@jdeppe-pivotal
Copy link
Copy Markdown
Contributor

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

  • Has your PR been rebased against the latest commit within the target branch (typically develop)?

  • Is your initial contribution a single, squashed commit?

  • Does gradlew build run cleanly?

  • Have you written or updated unit tests to verify your changes?

  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

Note:

Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.

params[1] = cqName;

final ResultCollector<?, ?> rc =
CliUtil.executeFunction(new CloseDurableCqFunction(), params, targetMembers);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we are trying to get away from using CliUtils in the command

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Properties props = new Properties();
props.setProperty(LOG_LEVEL, "fine");
props.setProperty(ConfigurationProperties.JMX_MANAGER_HOSTNAME_FOR_CLIENTS, "localhost");
props.setProperty(ConfigurationProperties.JMX_MANAGER_PORT, "" + jmxPort);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think these properties are needed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes - removed.

String commandString = csb.toString();

gfsh.executeAndAssertThat(commandString).statusIsError()
.tableHasColumnWithExactValuesInAnyOrder("CQ Name",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should deprecate these methods in CommandResultAssert. these are used for the old CommandResult, for the ResultModel, the other assertions are much easier to understand.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@jdeppe-pivotal jdeppe-pivotal force-pushed the feature/GEODE-5971-refactor-durable-client-commands branch from de8c79a to 031b7c2 Compare November 15, 2018 21:54
@jdeppe-pivotal jdeppe-pivotal merged commit 6371496 into apache:develop Nov 16, 2018
@jdeppe-pivotal jdeppe-pivotal deleted the feature/GEODE-5971-refactor-durable-client-commands branch November 16, 2018 14:07
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.

2 participants