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.

Sign up for GitHub

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

Jump to bottom

Save to custom table node: improvements #10581

Open
wants to merge 7 commits into
base: develop/3.7.1
Choose a base branch
from
Open

Save to custom table node: improvements #10581

wants to merge 7 commits into from

Conversation

irynamatveieva
Copy link
Contributor

@irynamatveieva irynamatveieva commented Apr 16, 2024 •
edited

Pull Request description

  • added tests for Save to custom table node
  • added ability to define ttl via configuration
  • added tests for new functionality

General checklist

  • You have reviewed the guidelines document.
  • Labels that classify your pull request have been added.
  • The milestone is specified and corresponds to fix version.
  • Description references specific issue.
  • Description contains human-readable scope of changes.
  • Description contains brief notes about what needs to be added to the documentation.
  • No merge conflicts, commented blocks of code, code formatting issues.
  • Changes are backward compatible or upgrade script is provided.
  • Similar PR is opened for PE version to simplify merge. Crosslinks between PRs added. Required for internal contributors only.

Back-End feature checklist

  • Added corresponding unit and/or integration test(s). Provide written explanation in the PR description if you have failed to add tests.
  • If new dependency was added: the dependency tree is checked for conflicts.
  • If new service was added: the service is marked with corresponding @TbCoreComponent, @TbRuleEngineComponent, @TbTransportComponent, etc.
  • If new REST API was added: the RestClient.java was updated, issue for Python REST client is created.
  • If new yml property was added: make sure a description is added (above or near the property).

@ashvayka ashvayka added this to the 3.7.1 milestone May 6, 2024
@ShvaykaD ShvaykaD added enhancement Rule Engine Changes to Rule Engine labels May 31, 2024
@irynamatveieva irynamatveieva changed the base branch from master to develop/3.7.1 June 3, 2024 09:21
ShvaykaD
ShvaykaD reviewed Jun 7, 2024
View reviewed changes
...s/src/test/java/org/thingsboard/rule/engine/action/TbSaveToCustomCassandraTableNodeTest.java Outdated
void givenCassandraClusterIsMissing_whenInit_thenThrowsException() {
var configuration = new TbNodeConfiguration(JacksonUtil.valueToTree(config));
assertThatThrownBy(() -> node.init(ctxMock, configuration))
.isInstanceOf(RuntimeException.class)
Copy link
Contributor

@ShvaykaD ShvaykaD Jun 7, 2024 •
edited

Choose a reason for hiding this comment

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

By default, TB tries to init the node 10 times before stopping the actor. For cases when the issue is unrecoverable, we use TbNodeException with unrecoverable flag set to true.

Could you please check if this issue might be recoverable? Try to stop the Cassandra cluster and after that initiate the node. After a certain number of retry attempts, start the Cassandra cluster back. If the node will be started then change the exception to TbNodeException with unrecoverable flag set to false, otherwise set it to true.

ShvaykaD
ShvaykaD reviewed Jun 7, 2024
View reviewed changes
...s/src/test/java/org/thingsboard/rule/engine/action/TbSaveToCustomCassandraTableNodeTest.java Outdated
TbMsg msg = TbMsg.newMsg(TbMsgType.POST_TELEMETRY_REQUEST, DEVICE_ID, TbMsgMetaData.EMPTY, TbMsg.EMPTY_STRING);
assertThatThrownBy(() -> node.onMsg(ctxMock, msg))
.isInstanceOf(IllegalStateException.class)
.hasMessage("Invalid message structure, it is not a JSON Object:" + null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.hasMessage("Invalid message structure, it is not a JSON Object:" + null);
.hasMessage("Invalid message structure, it is not a JSON Object: " + null);

ShvaykaD
ShvaykaD reviewed Jun 7, 2024
View reviewed changes
...s/src/test/java/org/thingsboard/rule/engine/action/TbSaveToCustomCassandraTableNodeTest.java Outdated
node.onMsg(ctxMock, msg);

verify(sessionMock).prepare("INSERT INTO cs_tb_readings(entityId) VALUES(?)");
verify(boundStatementBuilderMock).setUuid(anyInt(), eq(DEVICE_ID.getId()));
Copy link
Contributor

@ShvaykaD ShvaykaD Jun 7, 2024 •
edited

Choose a reason for hiding this comment

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

why anyInt()?

ShvaykaD
ShvaykaD reviewed Jun 7, 2024
View reviewed changes
...s/src/test/java/org/thingsboard/rule/engine/action/TbSaveToCustomCassandraTableNodeTest.java Outdated
Consumer<BoundStatementBuilder> mockBuilderConsumer,
Consumer<BoundStatementBuilder> verifyBuilderConsumer) throws TbNodeException {
config.setTableName("sensor_readings");
config.setFieldsMapping(Map.of("msgField1", "tableColumn1", "msgField2", "tableColumn2"));
Copy link
Contributor

Choose a reason for hiding this comment

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

why we need msgField2 in this test? Is it used in the test somehow? Maybe it's better to add as many fields as you supported data types and replace the parameterized test then?

@irynamatveieva irynamatveieva changed the title [WIP] Save to custom table node: added tests Save to custom table node: improvements Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShvaykaD ShvaykaD ShvaykaD left review comments

Assignees
No one assigned
Labels
enhancement Rule Engine Changes to Rule Engine
Projects
None yet
Milestone
3.7.1
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@irynamatveieva @ShvaykaD @ashvayka