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

Added option to turn shared subscription off or change the group identifier #754

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fbuedding
Copy link

Twin PR, same changes.

if (
amqpConfig.options && amqpConfig.options.durable
) {
if (amqpConfig.options && amqpConfig.options.durable) {
Copy link
Member

Choose a reason for hiding this comment

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

the code of this file was tainted, but no really changes?

Copy link
Author

@fbuedding fbuedding Sep 27, 2023

Choose a reason for hiding this comment

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

This is due linting and/or prettier. I just ran the scripts defined in the package.json

@@ -74,15 +74,15 @@ function serializedPayloadCommand(payload, command) {
*/
function generateCommandExecution(apiKey, device, attribute) {
let payload = {};
let command = device && device.commands.find((att) => att.name === attribute.name);
const command = device && device.commands.find((att) => att.name === attribute.name);
Copy link
Member

@AlvaroVega AlvaroVega Sep 27, 2023

Choose a reason for hiding this comment

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

command could be modified in line 99

Copy link
Author

Choose a reason for hiding this comment

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

Also linitng and prettier

@AlvaroVega
Copy link
Member

AlvaroVega commented Sep 27, 2023

Maybe new configuration option about disable shared subscription should be also added an explained into: https://github.com/telefonicaid/iotagent-json/blob/master/docs/installationguide.md#mqtt-configuration
In which cases disable shared subscription is a good idea?

@fbuedding
Copy link
Author

As described in the twin PR I only really changed the following files:

  • test/unit/startup-test.js
  • test/unit/ngsiv2/configurationsMqtt-test.js
  • lib/configService.js
  • lib/bindings/MQTTBinding.js
  • docs/usermanual.md
  • docs/installationguide.md

topics.push(constants.MQTT_SHARE_SUBSCRIPTION_GROUP + '/+/+/' + constants.MEASURES_SUFIX + '/+');
topics.push(
constants.MQTT_SHARE_SUBSCRIPTION_GROUP +
if (config.getConfig().mqtt.sharedSubscriptionsDisabled === true) {
Copy link
Member

Choose a reason for hiding this comment

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

@fbuedding
Copy link
Author

Honestly no idea why the test is failing or timing out, here are my test results

(16.x):

129 passing (13s)
3 pending

File % Stmts % Branch % Funcs % Lines Uncovered Line #s
All files 83.18 68.97 86.58 82.97
lib 86.46 82.12 85.89 86.21
commandHandler.js 90.47 72.22 100 90.16 46-47,59,153-161,165
commonBindings.js 96.49 85.71 100 96.33 70,304-306
configService.js 93.54 91.91 100 93.49 49-50,150,153,156,225,229,233
constants.js 100 100 100 100
errors.js 13.33 100 10 13.33 33-100
iotaUtils.js 83.33 73.01 100 83.13 46-47,58-59,118-119,172,179,197-208,224,228-237
iotagent-json.js 82.69 66.66 84.61 82.69 94,110,128,166-172
transportSelector.js 100 100 100 100
lib/bindings 80.11 56.84 87.2 79.96
AMQPBinding.js 76.85 43.47 88.88 76.85 75,78,84,90,98,105,110,118,121-123,135,142-145,149-150,156-159,176,214,259
HTTPBinding.js 77.94 62.25 86.66 77.69 ...-160,164,200,213,227-236,262-263,345,353,381,383,406,408,418,425,440-448,523,555,562-565,571,579,593-597,658-663,681-705
MQTTBinding.js 86.48 54.54 86.95 86.39 141-143,169,172,194,232,235,267,276-279,284-286,295,298,323,401,423

(18.x)

129 passing (13s)
3 pending

File % Stmts % Branch % Funcs % Lines Uncovered Line #s
All files 83.38 68.97 86.58 83.16
lib 86.46 82.12 85.89 86.21
commandHandler.js 90.47 72.22 100 90.16 46-47,59,153-161,165
commonBindings.js 96.49 85.71 100 96.33 70,304-306
configService.js 93.54 91.91 100 93.49 49-50,150,153,156,225,229,233
constants.js 100 100 100 100
errors.js 13.33 100 10 13.33 33-100
iotaUtils.js 83.33 73.01 100 83.13 46-47,58-59,118-119,172,179,197-208,224,228-237
iotagent-json.js 82.69 66.66 84.61 82.69 94,110,128,166-172
transportSelector.js 100 100 100 100
lib/bindings 80.49 56.84 87.2 80.34
AMQPBinding.js 76.85 43.47 88.88 76.85 75,78,84,90,98,105,110,118,121-123,135,142-145,149-150,156-159,176,214,259
HTTPBinding.js 77.94 62.25 86.66 77.69 ...-160,164,200,213,227-236,262-263,345,353,381,383,406,408,418,425,440-448,523,555,562-565,571,579,593-597,658-663,681-705
MQTTBinding.js 87.83 54.54 86.95 87.75 169,172,194,208,232,235,267,276-279,284-286,295,298,323,401,423

@AlvaroVega AlvaroVega changed the title Twin PR for iotagent-ul Added option to turn shared subscription off or change the group identifier Oct 5, 2023
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