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

re-add getDeviceById #1625

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

Conversation

jason-fox
Copy link
Contributor

Fixes #1624

Checking with NGSI-v2, 4c44db3 is fine, 964ee27 onwards manifests an incorrect payload as:

 "undefined": {
	"type": "None",
	"value": null,
	"metadata": {
	"TimeInstant": {
	    "type": "DateTime",
	    "value": "2024-06-27T09:28:53.963Z"
	}
}

The only config functions that has changed at this point is getDevice() where the fallback to getDeviceById() had been removed

function getDevice(id, apikey, service, subservice, callback) {
    getDeviceById(id, apikey, service, subservice, function (error, data) {
        if (error) {
+            // Try without apikey: apikey will be added to device later
+           getDeviceById(id, null, service, subservice, function (error, data) {
+                if (error) {
+                    callback(error);
+               } else {
+                    callback(null, data);
+               }
+            });
-            callback(error);
        } else {
            callback(null, data);
        }
    });
}

Re-adding the code fixes this direct issue - although linked entities are still not present in NGSI-LD

@jason-fox
Copy link
Contributor Author

@AlvaroVega - Since the apikey service plus apikey on device workaround works ✅ - it is up to you to decide how to proceed here.

Either accept this PR to maintain backwards compatibility, or reject the PR and properly document this change of behaviour as a breaking change.

@AlvaroVega
Copy link
Member

But with this is not possible define two devices in de same service/subservice with the same id but different apikey.

@jason-fox
Copy link
Contributor Author

jason-fox commented Jun 28, 2024

That still leaves you with two choices 😄 :

  1. Document the breaking change as a breaking change.
  2. Add a start-up parameter or ENV variable to either:
    • maintain backward compatibility (as in the PR)
    • allow define two devices same id different apiKey

I would assume the breaking change path is more likely to annoy existing users who are looking to upgrade, but the flag for old functionality is a maintenance debt.

At least if I'm upgrading from 4.3.0 to 5.0.0 I'm more likely to throughly test the setup and more likely to look at the release note.

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.

Config from Device API not used.
2 participants