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

Unify config path usage in RESTAPI (and all of Tuktu) #39

Open
NTPape opened this issue Jul 20, 2016 · 1 comment
Open

Unify config path usage in RESTAPI (and all of Tuktu) #39

NTPape opened this issue Jul 20, 2016 · 1 comment

Comments

@NTPape
Copy link
Collaborator

NTPape commented Jul 20, 2016

  1. Some actions require the config path to be stripped of its .json file extension (e.g. starting / stopping flows by config path), some require it to be present (e.g. storing new configs / removing configs).
    I would argue the file extension should be present everywhere (maybe only strip it in the frontend for readability).
  2. Additionally, for some actions a preceding '/' in front of the path is optional (e.g. storing new configs / removing configs), others need to be consistent within itself (starting / stopping flows by config name), while the automatically started analytics scripts have it, so if you argue for consistency you could say that it's required.
    Again, I would argue we should normalize paths, that includes file extensions as mentioned above, and slashes (or no slashes) in front of the path. Could use Java 8's Paths to do so.
  3. Another aspect where it is not unified is with REST API urls, for some it's part of the query parameters, with others it's not.
    Here I would argue that it should not be part of the query parameters, but always be part of the request's content.
@NTPape
Copy link
Collaborator Author

NTPape commented Aug 1, 2016

You closed the wrong issue.

@NTPape NTPape reopened this Aug 1, 2016
NTPape added a commit that referenced this issue Nov 16, 2016
- getConfig API call now also supports path with .json ending; #39
- Added utility method to load config file from configs folder
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant