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
Read password and username via file from MongoDB datasource #1168
Read password and username via file from MongoDB datasource #1168
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work. Left a trivial comment, while looks pretty neat entirely. Thank you!
pkg/datastore/mongodb/mongodb.go
Outdated
const ( | ||
// Scram type ref https://github.com/mongodb/mongo-go-driver/blob/9e2aca8afd8821e6b068cc2f25192bc640d90a0d/mongo/client_examples_test.go#L119 | ||
// Default | ||
Scram string = "SCRAM" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I feel like we can keep it private until needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nakabonne Thanks for your review, I change it to unexported one. Please check again when you have time 👍 👍
/trigger presubmit |
/trigger presubmits |
@nakabonne: Your requested presubmits has been scheduled in response to this comment. |
} | ||
return mongodb.NewMongoDB(ctx, mdConfig.URL, | ||
mdConfig.Database, | ||
options...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Just want to be unified with our coding conventions.
mongodb.NewMongoDB(
ctx,
mdConfig.URL,
...,
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nghialv
Thanks for your review.
I changed it to match the coding conventions. Please check it again when you have time 👍 👍
/lgtm |
/trigger presubmits |
@nakabonne: Your requested presubmits has been scheduled in response to this comment. |
@tnqv Thank you. You're the best! |
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #683
Does this PR introduce a user-facing change?: