feat: add db connection configuration #254
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
value to error
singhvikash11
requested review from
ravisuhag,
rahmatrhd and
AkarshSatija
as code owners
Pull Request Test Coverage Report for Build 2838249550
💛 - Coveralls |
singhvikash11
force-pushed
the
conn_config
branch
from
d014250
to
6b4d7ba
Compare
rahmatrhd
requested changes
Aug 11, 2022
| @@ -41,6 +41,15 @@ func NewStore(c *store.Config) (*Store, error) { | |||
| log.Panic(err) | |||
| } | |||
|
|
|||
| db, err := gormDB.DB() | |||
| if err != nil { | |||
| log.Panic(err) | |||
There was a problem hiding this comment.
Suggested change
| log.Panic(err) | |
| return nil, err |
|
@singhvikash11 these changes are present in the latest version of salt. Can we bump up the salt version and use that? |
mabdh
reviewed
Sep 21, 2022
| SslMode string `mapstructure:"sslmode" default:"disable"` | ||
| MaxIdleConns int `yaml:"max_idle_conns" mapstructure:"max_idle_conns" default:"3"` | ||
| MaxOpenConns int `yaml:"max_open_conns" mapstructure:"max_open_conns" default:"10"` | ||
| ConnMaxLifeTime time.Duration `yaml:"conn_max_life_time" mapstructure:"conn_max_life_time" default:"10ms"` |
There was a problem hiding this comment.
For the clarity better to add the unit itself.
Suggested change
| ConnMaxLifeTime time.Duration `yaml:"conn_max_life_time" mapstructure:"conn_max_life_time" default:"10ms"` | |
| ConnMaxLifeTimeMS time.Duration `yaml:"conn_max_life_time_ms" mapstructure:"conn_max_life_time" default:"10ms"` |
mabdh
reviewed
Sep 21, 2022
| @@ -1,12 +1,18 @@ | |||
| package store | |||
|
|
|||
There was a problem hiding this comment.
we are moving from gorm and I think we suppose to use salt/db
|
@singhvikash11 +1, we plan to remove Gorm. So salt/db package will be used for this. If any of the missing setting is not there then lets raise PR for that in salt. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Address connection setting