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

Change configuration group operations URL to some more meaninfull #752

Open
fgalan opened this issue Jan 25, 2019 · 7 comments
Open

Change configuration group operations URL to some more meaninfull #752

fgalan opened this issue Jan 25, 2019 · 7 comments
Labels

Comments

@fgalan
Copy link
Member

fgalan commented Jan 25, 2019

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
Copy link
Member Author

fgalan commented Apr 29, 2019

Addressed by PR #780 #922 #954

@Madhu1029
Copy link

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?

@mapedraza
Copy link
Collaborator

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:

@fgalan
Copy link
Member Author

fgalan commented Mar 14, 2023

@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 Added Feature/config groups #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 Added Feature/config groups #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 #1314 gets merged. Otherwise, you would be working with a code base that soons will become obsolete.

@Madhu1029
Copy link

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.

@fgalan
Copy link
Member Author

fgalan commented Mar 23, 2023

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
Copy link
Member Author

fgalan commented Mar 23, 2023

As the PR #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants