Skip to content

Add NGSI-LD multi-measures support #847

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
Merged
fgalan merged 19 commits into from
Mar 4, 2020
Merged

Add NGSI-LD multi-measures support #847

fgalan merged 19 commits into from
Mar 4, 2020

Conversation

@jason-fox
Copy link
Contributor

@jason-fox jason-fox commented Feb 25, 2020

  • split NGSI-v1, NGSI-v2 and NGSI-LD into separate files
  • Move common REST functions into restUtils.js
  • ensure that ids are URNS (adding urn:ngsi-ld: where necessary)
  • Use Batch Upsert endpoint throughout ( /ngsi-ld/v1/entityOperations/upsert/) instead of PATCH update
  • Update test expectations
  • Split NGSI v2 and NGSI-LD registrations

@jason-fox
Add multimeasures support
bd63653 
- split NGSI-v1, NGSI-v2 and NGSI-LD into separate files
- Move common REST functions into restUtils.js
- ensure that ids are URNS (adding urn:ngsi-ld: where necessary)
- Use Batch Upsert endpoint throughout ( `/ngsi-ld/v1/entityOperations/upsert/`) instead of PATCH update
- Update test expectations
- Split NGSI v2 and NGSI-LD registrations
@jason-fox jason-fox mentioned this pull request Feb 25, 2020
Closed
@jason-fox
Fixing lint errors
1234ede
@jason-fox
Formatting tests as ES6
4ad7b8a
@jason-fox
Capitalize file names
dfd0be0
@jason-fox
Capitalize file names
8ea1221
@jason-fox
Enable travis
b0943b2
@jason-fox
Capitalize file path to fix Coverage
93bf97b
@jason-fox
Capitalize file path to fix Coverage
c836260
@jason-fox
Changes after review. (feature/ngsi-ld-measure)
bebf658 
-  Update CNR
-  Add Docker ENV to docs
-  basic LD set-up documentation
-  amend copy-paste JavaDoc
-  add missing JavaDoc
-  remove blank lines.
@jason-fox
Merge branch 'feature/time-and-geo' into feature/ngsi-ld-multimeasure
78c9f59 
Amend JavaDoc
@jason-fox
Linting. Tidy Whitespace on modified files
75467bb
@jason-fox
Modify indent
603e024
@jason-fox
More JavaDoc NGSI-v2 => NSGI-LD
11742cc 
Merge branch 'feature/time-and-geo' into feature/ngsi-ld-multimeasure
@jason-fox
Merge branch 'feature/time-and-geo' into feature/ngsi-ld-multimeasure
2de60eb
@jason-fox
Merge branch 'feature/time-and-geo' into feature/ngsi-ld-multimeasure
74e42ab
@fgalan fgalan changed the base branch from master to feature/842_ngsi_ld March 4, 2020 11:26
@fgalan
Copy link
Member

fgalan commented Mar 4, 2020

PR #843 has been merged and base branch changed to feature/842_ngsi_ld in this PR.

So I understand you should upgrade jason-fox:feature/ngsi-ld-multimeasure with telefonicaid:feature/842_ngsi_ld

fgalan
fgalan reviewed Mar 4, 2020
View reviewed changes
lib/services/devices/registrationUtils.js
);
}

//console.error(JSON.stringify(options.json, null, 4));
Copy link
Member

@fgalan fgalan Mar 4, 2020

Choose a reason for hiding this comment

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

Leftover comment?

Copy link
Contributor Author

@jason-fox jason-fox Mar 4, 2020

Choose a reason for hiding this comment

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

Yes, but - the multiple instances of console.error (commented out) are being used to help generate the responses for the tests.

When the final PR is processed I'll remove them all.

Copy link
Member

@fgalan fgalan Mar 4, 2020

Choose a reason for hiding this comment

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

Noted in the accumulative PR - #849 (comment)

NTC with regards this PR

fgalan
fgalan reviewed Mar 4, 2020
View reviewed changes
@fgalan
Copy link
Member

fgalan commented Mar 4, 2020

I think the PR is ok (except for a couple of very minor comments easy to fix)

However, regarding:

split NGSI-v1, NGSI-v2 and NGSI-LD into separate files

I like the refactor to move separate parts into -NGSI-LD.js -NGSI-v1.js and- NGSI-v2.js files in different parts. However, I wonder about the usual question: which impact could have this change into other existing PRs? Could you analyze it a little bit and provide feedback, pls?

@jason-fox
Copy link
Contributor Author

jason-fox commented Mar 4, 2020 •
edited
Loading

split NGSI-v1, NGSI-v2 and NGSI-LD into separate files

This is very simple - All of the split /devices, /ngsi and /northBound files are to do with NGSI Interactions only - these are all defined by the relevant specifications, so there is zero likelihood that another PR will be amending them. It will have NO impact on the other PRs. The split has not changed the functionality of the underlying code (since none of the v1 or v2 tests has been modified) - its merely cut-paste rearrangement.

Going forward, reducing the file size means that it is even less likely that a clash will occur - besides which it is practically impossible to read a single 2000+ line file with interleaving implementations of three near duplicates of each function with only fractional differences between them. [sigh]

It also allows for:

  • the v1/v2/ld flag checking to occur once only - see jason-fox@2f2caea. (later PR)
  • a comparison to be made between the code coverage for v1, v2 and LD in coveralls.

@jason-fox
Copy link
Contributor Author

jason-fox commented Mar 4, 2020 •
edited
Loading

Note that things like #835 can touch files like deviceService.js, but this is common functionality which applies to v1 v2 and LD and therefore the relevant functions (e.g. registerDevice(), prepareDeviceData() ) remain unchanged in my PR and in place in the root file.

@fgalan fgalan mentioned this pull request Mar 4, 2020
Merged
fgalan
fgalan approved these changes Mar 4, 2020
View reviewed changes
Copy link
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.

Ok. The rationale is sound :)

LGTM

@fgalan fgalan merged commit 8399d52 into telefonicaid:feature/842_ngsi_ld Mar 4, 2020
@fgalan fgalan mentioned this pull request Mar 4, 2020
Merged
@jason-fox jason-fox deleted the feature/ngsi-ld-multimeasure branch March 4, 2020 19:17
@jason-fox jason-fox mentioned this pull request Mar 10, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fgalan fgalan fgalan approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jason-fox @fgalan