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

feat(pull-mode): adds file discovery mechanism #662

Merged
merged 4 commits into from
Dec 29, 2021

Conversation

linthan
Copy link
Contributor

@linthan linthan commented Dec 27, 2021

add this feature.
#640

@CLAassistant
Copy link

CLAassistant commented Dec 27, 2021

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@kolesnikovae kolesnikovae left a comment

Choose a reason for hiding this comment

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

Thank you very much @linthan! This is awesome! I've left a couple of minor notes – please take a look.

pkg/scrape/discovery/file/file.go Outdated Show resolved Hide resolved
pkg/scrape/discovery/file/file.go Outdated Show resolved Hide resolved
pkg/scrape/discovery/file/file.go Outdated Show resolved Hide resolved
pkg/cli/server.go Outdated Show resolved Hide resolved
@kolesnikovae kolesnikovae added backend Mostly go code enhancement New feature or request labels Dec 27, 2021
@linthan
Copy link
Contributor Author

linthan commented Dec 27, 2021

hank you very much @linthan! This is awesome! I've left a couple of minor notes – please take a look.
ok, I will check those notes and solve those problems.

@codecov
Copy link

codecov bot commented Dec 27, 2021

Codecov Report

Merging #662 (d7a09a2) into main (6c390b5) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #662   +/-   ##
=======================================
  Coverage   75.76%   75.76%           
=======================================
  Files          45       45           
  Lines        1588     1588           
  Branches      292      292           
=======================================
  Hits         1203     1203           
  Misses        356      356           
  Partials       29       29           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c390b5...d7a09a2. Read the comment docs.

Copy link
Collaborator

@kolesnikovae kolesnikovae left a comment

Choose a reason for hiding this comment

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

Looks good! Please run go mod tidy and commit go.mod and go.sum.


// NewDiscovery returns a new file discovery for the given paths.
func NewDiscovery(conf *SDConfig, logger logrus.FieldLogger) *Discovery {

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess, revive linter complains about this empty line.

Copy link
Member

@petethepig petethepig left a comment

Choose a reason for hiding this comment

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

Looks awesome, thank you @linthan !

@petethepig petethepig changed the title add file discovery feat(pull-mode): adds file discovery mechanism Dec 28, 2021
@petethepig
Copy link
Member

@linthan Could you please remove that extra line after func NewDiscovery? I can merge after that.

@linthan
Copy link
Contributor Author

linthan commented Dec 29, 2021

@petethepig ok ,I will solve this.

@petethepig petethepig merged commit 35ce0b5 into grafana:main Dec 29, 2021
kolesnikovae pushed a commit that referenced this pull request Jan 7, 2022
* add file discovery

* fix log error and deal with  kebab-case config  rules

* gomod

* remove extra line to pass go-lint check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Mostly go code enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants