Skip to content
This repository has been archived by the owner on Aug 24, 2022. It is now read-only.

PMM-4879 Adding support for defaults-file. #356

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

Conversation

pkadej
Copy link
Contributor

@pkadej pkadej commented Apr 13, 2022

PMM-4879

Build: SUBMODULES-2465

Build will fail because I cannot use forked version of api (pmm) as dependency in mod.go. To acomplish this, api needs be merged and version of pmm api should be bumped.

@it-percona-cla
Copy link

it-percona-cla commented Apr 13, 2022

CLA assistant check
All committers have signed the CLA.

}

// ParseDefaultsFile parses given defaultsFile. It returns the database specs.
func (d *DefaultsFile) ParseDefaultsFile(req *agentpb.ParseDefaultsFileRequest) *agentpb.ParseDefaultsFileResponse {
Copy link
Member

Choose a reason for hiding this comment

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

let's create one more struct like DefaultsFileParser with no fields and make this method part of it.
because TBH it looks weird that DefaultsFile has method which calls another function which returns DefaultsFile and fields of it used. So I think having separate struct will make code cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.
Just created "Parser" struct because package is "defaultsfile" and with DefaultsFileParser linter raises error about stuttering. type name will be used as defaultsfile.DefaultsFileParser by other packages, and that stutters

)

// DefaultsFile is a struct with database specs fetched from file.
type DefaultsFile struct {
Copy link
Member

Choose a reason for hiding this comment

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

in case of new struct DefaultFileParser, DefaultsFile can be private.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great remark. Thank you, changed.

defaultsfile/defaults_file.go Show resolved Hide resolved

return &defaultsFile{
username: cfgSection.Key("user").String(),
password: cfgSection.Key("password").String(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Later password need to be encrypted, please create related ticket so that we don't forget about it

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants