Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

Refactor api and add v2 API (beta) #1489

Merged
merged 7 commits into from
Jan 26, 2017

Conversation

kindermoumoute
Copy link
Contributor

@kindermoumoute kindermoumoute commented Jan 24, 2017

Summary of changes:

  • @kjlyon's work on API tests.
  • Add the API interface
  • Move v1 under mgmt/rest/v1
  • Implements v2

@intelsdi-x/snap-maintainers

kjlyon and others added 3 commits October 4, 2016 15:09
        - V2 metrics routes now have a metrics section in the body of the response.
        - Fixed text case in v2/tests from "ScheduledTasks" to "scheduled_tasks" to be consistent with other responses.
        - Added V2 folders in rest and in rbody to hold v2 methods
- Made all v2 tribe routes to be /v2/tribes (made tribe plural)
- Changed V2/tribes/member/:name to  be V2/tribes/members/:name (made member plural)
Created V2 REST API Routes in feature branch snap/v2_rest_api
@kindermoumoute
Copy link
Contributor Author

The review of this PR has been done in #1474, this PR is just a rebase in order to have clear commits in the log.

@kindermoumoute kindermoumoute mentioned this pull request Jan 25, 2017
@kindermoumoute kindermoumoute changed the title Refactor api Refactor api and add v2 API (beta) Jan 25, 2017
@kindermoumoute kindermoumoute force-pushed the refactor_API branch 3 times, most recently from ccc51c0 to 15c1de2 Compare January 25, 2017 22:11
Adds version 2 of the rest api in order to have more consistent
responses, errors, and conform to REST better.

- Add package v2/mock
- Add medium tests for v2
- Implement API interface for v2
- Implement v2 routes
)

const (
ErrPluginAlreadyLoaded = "plugin is already loaded"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's take this opportunity to make the errors consistent in terms of language, punctuation and capitalization.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

const (
ErrPluginAlreadyLoaded = "plugin is already loaded"
ErrTaskNotFound = "Task not found"
ErrTaskDisabledNotRunnable = "Task is disabled. Cannot be started"
Copy link
Collaborator

Choose a reason for hiding this comment

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

"Cannot be started" can probably be dropped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@jcooklin
Copy link
Collaborator

@IRCody, @nanliu, @candysmurf can we get another LGTM?

@candysmurf
Copy link
Contributor

LGTM 👍

@jcooklin jcooklin merged commit 8080135 into intelsdi-x:master Jan 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants