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

Initial API v1 implementation #774

Merged
merged 4 commits into from
Jun 8, 2015
Merged

Initial API v1 implementation #774

merged 4 commits into from
Jun 8, 2015

Conversation

fabxc
Copy link
Contributor

@fabxc fabxc commented Jun 4, 2015

These commits add an initial API v1 that provides the equivalent endpoint to the old API accepting unix or RFC3339 timestamps.
Both APIs are running in parallel.

"github.com/prometheus/prometheus/web/api"

api_legacy "github.com/prometheus/prometheus/web/api/legacy"
api_v1 "github.com/prometheus/prometheus/web/api/v1"
Copy link
Member

Choose a reason for hiding this comment

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

Avoid underscore in names.

@fabxc fabxc force-pushed the fabxc/api-v1 branch 2 times, most recently from ce6000f to 9f839ab Compare June 6, 2015 19:51
"github.com/prometheus/prometheus/web/api"

legacy "github.com/prometheus/prometheus/web/api/legacy"
v1 "github.com/prometheus/prometheus/web/api/v1"
Copy link
Member

Choose a reason for hiding this comment

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

The renaming is not even required in that case. The package is called v1 anyway.

@beorn7
Copy link
Member

beorn7 commented Jun 8, 2015

Perhaps rename package api into package legacy, which would match the directory name.
Then, none of the named imports are necessary.

Who else is about to review this? @juliusv ?

@@ -41,7 +41,9 @@ import (
"github.com/prometheus/prometheus/storage/remote/influxdb"
"github.com/prometheus/prometheus/storage/remote/opentsdb"
"github.com/prometheus/prometheus/web"
"github.com/prometheus/prometheus/web/api"

Copy link
Member

Choose a reason for hiding this comment

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

There should be no newline here as per our import group conventions...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@juliusv
Copy link
Member

juliusv commented Jun 8, 2015

👍, but note my latest two comments on the design doc (ok to encode timestamp/values as tuples also for vectors, and not needing a special endpoint for metrics names).

@fabxc
Copy link
Contributor Author

fabxc commented Jun 8, 2015

Merging these basics so we have the changes in web.go and main.go in master.
We can build on top of that.

fabxc added a commit that referenced this pull request Jun 8, 2015
@fabxc fabxc merged commit ae01a53 into master Jun 8, 2015
@fabxc fabxc deleted the fabxc/api-v1 branch June 8, 2015 17:14
simonpasquier pushed a commit to simonpasquier/prometheus that referenced this pull request Oct 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants