Skip to content

Conversation

@Wei-hao-Li
Copy link
Collaborator

No description provided.

Signed-off-by: Weihao Li <18110526956@163.com>
Signed-off-by: Weihao Li <18110526956@163.com>
Signed-off-by: Weihao Li <18110526956@163.com>
Signed-off-by: Weihao Li <18110526956@163.com>
Signed-off-by: Weihao Li <18110526956@163.com>
Signed-off-by: Weihao Li <18110526956@163.com>
Copy link
Contributor

@JackieTien97 JackieTien97 left a comment

Choose a reason for hiding this comment

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

partial review, finish CN and antlr parts.

Copy link
Contributor

@JackieTien97 JackieTien97 left a comment

Choose a reason for hiding this comment

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

Add all ITs you list in the documents and the recovery process is missing

ExternalServiceManagementService.getInstance().stopService(serviceName);
future.set(new ConfigTaskResult(new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode())));
} catch (Exception e) {
future.setException(new ExternalServiceManagementException(e.getMessage()));
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

ExternalServiceManagementService.getInstance().dropService(serviceName, forcedly);
future.set(new ConfigTaskResult(new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode())));
} catch (Exception e) {
future.setException(new ExternalServiceManagementException(e.getMessage()));
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

new ConfigTaskResult(
TSStatusCode.SUCCESS_STATUS, builder.build(), getShowExternalServiceHeader()));
} catch (Exception e) {
future.setException(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

Comment on lines +28 to +35
MQTT(
"MQTT",
"org.apache.iotdb.externalservice.Mqtt",
IoTDBDescriptor.getInstance().getConfig()::isEnableMQTTService),
REST(
"REST",
"org.apache.iotdb.externalservice.Rest",
IoTDBRestServiceDescriptor.getInstance().getConfig()::isEnableRestService);
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
MQTT(
"MQTT",
"org.apache.iotdb.externalservice.Mqtt",
IoTDBDescriptor.getInstance().getConfig()::isEnableMQTTService),
REST(
"REST",
"org.apache.iotdb.externalservice.Rest",
IoTDBRestServiceDescriptor.getInstance().getConfig()::isEnableRestService);
MQTT(
"MQTT",
"org.apache.iotdb.externalservice.Mqtt",
// IoTDBDescriptor.getInstance().getConfig()::isEnableMQTTService),
() -> false),
REST(
"REST",
"org.apache.iotdb.externalservice.Rest",
// IoTDBRestServiceDescriptor.getInstance().getConfig()::isEnableRestService);
() -> false);

better always return false before you really seperate the rest and mqtt service outside. Otherwise, mqtt and rest ITs will fail because org.apache.iotdb.externalservice.Mqtt and org.apache.iotdb.externalservice.Rest is not found

Comment on lines +28 to +35
MQTT(
"MQTT",
"org.apache.iotdb.externalservice.Mqtt",
IoTDBDescriptor.getInstance().getConfig()::isEnableMQTTService),
REST(
"REST",
"org.apache.iotdb.externalservice.Rest",
IoTDBRestServiceDescriptor.getInstance().getConfig()::isEnableRestService);
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
MQTT(
"MQTT",
"org.apache.iotdb.externalservice.Mqtt",
IoTDBDescriptor.getInstance().getConfig()::isEnableMQTTService),
REST(
"REST",
"org.apache.iotdb.externalservice.Rest",
IoTDBRestServiceDescriptor.getInstance().getConfig()::isEnableRestService);
MQTT(
"MQTT",
"org.apache.iotdb.externalservice.Mqtt",
// IoTDBDescriptor.getInstance().getConfig()::isEnableMQTTService),
() -> false),
REST(
"REST",
"org.apache.iotdb.externalservice.Rest",
// IoTDBRestServiceDescriptor.getInstance().getConfig()::isEnableRestService);
() -> false);

better always return false before you really seperate the rest and mqtt service outside. Otherwise, mqtt and rest ITs will fail because org.apache.iotdb.externalservice.Mqtt and org.apache.iotdb.externalservice.Rest is not found

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