Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: add influx support(not implement test case) and has some todo item #281

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Belyenochi
Copy link

What type of PR is this?

support influxDB, solved #119

@Belyenochi
Copy link
Author

To be implemented test case, it is expected to initiate a formal merger tomorrow. @yufeiyu

@qmhu qmhu added this to the 0.5.0 milestone Apr 25, 2022
@yufeiyu
Copy link
Contributor

yufeiyu commented Apr 25, 2022

Please make sure all checks passed.

@@ -3,6 +3,7 @@ package app
import (
"context"
"flag"
"github.com/gocrane/crane/pkg/providers/influxdb"
Copy link
Member

Choose a reason for hiding this comment

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

import group

package influxdb

import (
gocontext "context"
Copy link
Member

Choose a reason for hiding this comment

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

import group

Comment on lines +3 to +11
import (
gocontext "context"
"github.com/gocrane/crane/pkg/common"
"github.com/gocrane/crane/pkg/metricnaming"
"github.com/gocrane/crane/pkg/metricquery"
"github.com/gocrane/crane/pkg/providers"
"k8s.io/klog/v2"
"time"
)

Choose a reason for hiding this comment

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

Suggeset to follow a policy of grouping imports in 3~4 groups,e.g.:

import (
    build-in-stdlib

    current project/repo

    all others 
)

prefer:

import (
        "time"
	gocontext "context"

	"github.com/gocrane/crane/pkg/common"
	"github.com/gocrane/crane/pkg/metricnaming"
	"github.com/gocrane/crane/pkg/metricquery"
	"github.com/gocrane/crane/pkg/providers"

	"k8s.io/klog/v2"
)

}


func (i *influxDB) QueryTimeSeries(namer metricnaming.MetricNamer, startTime time.Time, endTime time.Time, step time.Duration) ([]*common.TimeSeries, error) {

Choose a reason for hiding this comment

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

Exported type or method should have comment or be unexported:

// QueryTimeSeries  xxx xxx
func (i *influxDB) QueryTimeSeries(......

return timeSeries, nil
}

func (i *influxDB) QueryLatestTimeSeries(namer metricnaming.MetricNamer) ([]*common.TimeSeries, error) {

Choose a reason for hiding this comment

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

ditto. add comment

@Belyenochi
Copy link
Author

Sorry, I didn't fully finish this PR before, I will re-update this PR this weekend.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants