iotagent-node-lib icon indicating copy to clipboard operation
iotagent-node-lib copied to clipboard

Change configuration group operations URL to some more meaninfull

Open fgalan opened this issue 6 years ago • 7 comments

IOTAs implements a concept named "configuration group" but, unfortunatelly, the API operations to manage them are using "service" literal in the URL

  • POST /iot/services -> create a new configuration group
  • GET /iot/services -> get configuration group
  • etc.

This is very confusing, as service is another concept used for authorization, used in fiware-service header.

Thus, URL should be changed to something different and close to "configuration group" term, for instance:

  • POST /iot/configurations -> create a new configuration group
  • GET /iot/confgurations -> get configuration group
  • etc.

(Not sure if "configurations" is the best one, maybe there are shorter alternatives, e.g. "groups", "cgroups", etc.)

Some additional comments:

  • The changes should be backward compatible. I mean, although we change the URLs in code an documentation the old ones will be still valid.
  • The changes will be done in the unit tests, but some tests should be added using the old URLs to ensure backward compatibility is not broken (a subset of the old ones)

fgalan avatar Jan 25 '19 12:01 fgalan

Addressed by PR ~https://github.com/telefonicaid/iotagent-node-lib/pull/780~ ~#922~ #954

fgalan avatar Apr 29 '19 15:04 fgalan

Hi @fgalan , None of the PR has been merged yet to close this issue. Could you please let me know if the work is still required, i can make a new PR as per current code?

Madhu1029 avatar Mar 13 '23 07:03 Madhu1029

Hi @fgalan , None of the PR has been merged yet to close this issue. Could you please let me know if the work is still required, i can make a new PR as per current code?

There are 2 PRs opened with feedback given regarding this issue. You can find them here. If you are interested to contribute please address the review comments posted in the previous PRs:

  • https://github.com/telefonicaid/iotagent-node-lib/pull/954
  • https://github.com/telefonicaid/iotagent-node-lib/pull/1198

mapedraza avatar Mar 13 '23 12:03 mapedraza

@MadhuNEC the functionality is still valid. We want to implement the new API routes, and deprecate the old ones (although without removing them by the moment, due to backward compatibility reasons).

As @mapedraza mentions, previous work has been done in this issue. In particular:

  • PR #954, started on December 2020, without activity after June 2021, with a lot of comments (>50, not sure how many already addressed and how many pending) and a lot of conflicting files.
    • By the way, are you the same Madhu that authored that PR? (the github username in not exactly the same, so I doubt...)+
  • PR #1198, started on February 2022 and without activity since that months, also with a lot of comments (>25, and also not sure how many addressed and how many pending) and a lot of conflicting files.

@MadhuNEC what would be your proposal? Continuing the work on some of the existing PRs? Or start from scratch (again :)? In the second case, it would be great if you could have a look to the comments in the above PRs and take it into account in the new PR.

However, I don't recommend to take any action until the ongoing refactor in PR https://github.com/telefonicaid/iotagent-node-lib/pull/1314 gets merged. Otherwise, you would be working with a code base that soons will become obsolete.

fgalan avatar Mar 14 '23 16:03 fgalan

Hi @fgalan,

As the PR #1314 is merged now. Could you please confirm if the work on this PR can be started?

By the way, are you the same Madhu that authored that PR? (the github username in not exactly the same, so I doubt...)+

Yes, I am the same. My email has been updated so need to create new account.

@MadhuNEC what would be your proposal? Continuing the work on some of the existing PRs? Or start from scratch (again :)? In the second case, it would be great if you could have a look to the comments in the above PRs and take it into account in the new PR.

I am planning to start from scratch. I will have a look to the comments in the above PRs and will incorporate the same in new PR.

Madhu1029 avatar Mar 22 '23 05:03 Madhu1029

I am planning to start from scratch. I will have a look to the comments in the above PRs and will incorporate the same in new PR.

I think is a good plan. Thanks for your involvement! :)

fgalan avatar Mar 23 '23 09:03 fgalan

As the PR https://github.com/telefonicaid/iotagent-node-lib/pull/1314 is merged now. Could you please confirm if the work on this PR can be started?

I do confirm. The part touched by the ongoing IOTA Lib refactor (PR #1314 is part of it but more PRs will follow) it is not related with the route APIs part, so there shouldn't be any conflict.

fgalan avatar Mar 23 '23 10:03 fgalan