Skip to content

Add mixed mode#947

Merged
fgalan merged 11 commits into
telefonicaid:masterfrom
jason-fox:feature/mixed
Dec 2, 2020
Merged

Add mixed mode#947
fgalan merged 11 commits into
telefonicaid:masterfrom
jason-fox:feature/mixed

Conversation

@jason-fox
Copy link
Copy Markdown
Contributor

@jason-fox jason-fox commented Nov 26, 2020

This PR adds an additional "mixed" mode to the IoT Agent.

  1. Flow of control of existing IoT Agent- NGSI v2 context brokers is not changed (e.g. Orion Classic)
  2. Flow of control of existing IoT Agent- NGSI-LD context brokers is not changed (e.g Stellio, Scorpio)
  3. "Mixed" mode is a compromise, suggested here and committed to raising as a separate PR here

@fgalan fgalan mentioned this pull request Nov 26, 2020
@jason-fox
Copy link
Copy Markdown
Contributor Author

jason-fox commented Nov 26, 2020

Includes the usual Documentation and Unit Test Changes and appropriate code coverage.

Before reviewing, I'd suggest actioning #943 first so the builds aren't queuing on Travis for hours before each suggested change. I think Travis is also causing the timeout of one test.

@fgalan
Copy link
Copy Markdown
Member

fgalan commented Nov 27, 2020

Before reviewing, I'd suggest actioning #943 first so the builds aren't queuing on Travis for hours before each suggested change. I think Travis is also causing the timeout of one test.

Done. Please sync the PR with master to get the changes.

Comment thread doc/api.md Outdated
| `explicitAttrs` | `explicitAttrs` | Boolean value to support selective ignore of measures for device so that IOTA doesn’t progress. If not specified default is false. | `true/false` |

| `explicitAttrs` | `explicitAttrs` | Boolean value to support selective ignore of measures for device so that IOTA doesn’t progress. If not specified default is `false`. | `true/false` |
| `ngsiVersion` | `ngsiVersion` | optional string value used in mixed mode to switch between **NGSI-v2** and **NGSI-LD**payloads. The default is `v2`. | `v2/ld/mixed` |
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.

It seems markdown rendering is broken here.

imagen

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 : 9ad9268

Comment thread doc/api.md Outdated
| `explicitAttrs` | `explicitAttrs` | optional boolean value, to support selective ignore of measures so that IOTA doesn’t progress. If not specified default is false. |
| `expressionLanguage` | `expresionLanguage` | optional boolean value, to set expression language used to compute expressions, possible values are: legacy or jexl. When not set or wrongly set, `legacy` is used as default value. |
| `explicitAttrs` | `explicitAttrs` | optional boolean value, to support selective ignore of measures so that IOTA doesn’t progress. If not specified default is `false`. |
| `ngsiVersion` | `ngsiVersion` | optional string value used in mixed mode to switch between **NGSI-v2** and **NGSI-LD** payloads. The default is `v2`.
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'd suggest to enumerate the possible values this field can take.

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.

In addition, I'd suggest to add the following:

If IOTA doesn't run in mixed mode, then this field is ignored.

If I have understood correctly what mixed mode means :)

Copy link
Copy Markdown
Contributor Author

@jason-fox jason-fox Nov 30, 2020

Choose a reason for hiding this comment

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

Fixed 40b5256

Comment thread doc/installationguide.md
}
```

- If you want to support a "mixed" mode with both **NGSI-v2** and **NGSI-LD** (experimental):
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'd suggest to add an explanation of what mixed mode means. For instance:

In mixed mode the NGSI version to use is chosen at group or device provisioning time, instead of using an static setting for that. The ngsiVersion field in the provisioning API is used to specify this, at both group at device level (the device level overriding the group setting).

It is just an example, it can be improved, of course.

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 52cb089

Comment thread doc/api.md Outdated
| `expressionLanguage` | `expresionLanguage` | optional boolean value, to set expression language used to compute expressions, possible values are: legacy or jexl. When not set or wrongly set, legacy is used as default value. |
| `explicitAttrs` | `explicitAttrs` | Boolean value to support selective ignore of measures for device so that IOTA doesn’t progress. If not specified default is false. | `true/false` |
| `explicitAttrs` | `explicitAttrs` | Boolean value to support selective ignore of measures for device so that IOTA doesn’t progress. If not specified default is `false`. | `true/false` |
| `ngsiVersion` | `ngsiVersion` | optional string value used in mixed mode to switch between **NGSI-v2** and **NGSI-LD**payloads. The default is `v2`. | `v2/ld/mixed` |
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 see v2/ld/mixed as possible values. However, what does "mixed" means in this API? Or maybe it's a typo and only "v2/ld" are possible?

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 e142485

Comment thread doc/api.md Outdated
| `expressionLanguage` | `expresionLanguage` | optional boolean value, to set expression language used to compute expressions, possible values are: legacy or jexl. When not set or wrongly set, legacy is used as default value. |
| `explicitAttrs` | `explicitAttrs` | Boolean value to support selective ignore of measures for device so that IOTA doesn’t progress. If not specified default is false. | `true/false` |
| `explicitAttrs` | `explicitAttrs` | Boolean value to support selective ignore of measures for device so that IOTA doesn’t progress. If not specified default is `false`. | `true/false` |
| `ngsiVersion` | `ngsiVersion` | optional string value used in mixed mode to switch between **NGSI-v2** and **NGSI-LD**payloads. The default is `v2`. | `v2/ld`. |
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.

As we did with the device API I think "When not running in mixed mode, this field is ignored." should be added here.

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 1a4d4fd

Comment thread doc/api.md
@@ -90,8 +90,9 @@ correspondence between the API resource fields and the same fields in the databa
| `attributes` | `attributes` | list of common active attributes of the device. For each attribute, its `name` and `type` must be provided, additional `metadata` is optional. |
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'd suggest to add the mixed mode to the NGSI-LD feature sublist in CHANGES_NEXT_RELEASE. I mean:

Add basic NGSI-LD support as experimental feature (#842)
...
- Commands
- Mixed mode (based in ngsiVersion field in the provisioning API)

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 3dee480

Copy link
Copy Markdown
Member

@fgalan fgalan left a comment

Choose a reason for hiding this comment

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

LGTM

@fgalan fgalan merged commit 2835d6d into telefonicaid:master Dec 2, 2020
@jason-fox jason-fox deleted the feature/mixed branch February 5, 2021 13:43
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