-
Notifications
You must be signed in to change notification settings - Fork 2
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
use k6 cloud credentials #98
Conversation
734b84f
to
adfa019
Compare
Signed-off-by: Pablo Chacin <[email protected]>
adfa019
to
e7238d6
Compare
Signed-off-by: Pablo Chacin <[email protected]>
Signed-off-by: Pablo Chacin <[email protected]>
Collectors struct { | ||
Cloud struct { | ||
Token string `json:"token"` | ||
} `json:"cloud"` |
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.
More FYI and not sure that we actually need workaround that, but until the k6 v0.50 the structure was different and had more layers, like ext.loadimpact
was what currently cloud
is.
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.
I'll open an issue and we can discuss if this is required.
|
||
// loadConfig loads the k6 config file from the given path or the default location. | ||
// if using the default location and the file does not exist, it returns an empty config. | ||
func loadConfig(configPath string) (k6configFile, error) { |
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.
Will be it difficult to add the debug logs here for success path, which we can enable by increasing verbosity (-v
). Something like Resolved config:
/home/oleg/.config/k6/config.json` which could be useful for us if we need to investigate the things?
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.
That will require some changes in the code as we are not logging anything at the moment. I will open an issue an address this in a follow-up PR.
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.
LGTM
Fixes #93
Fixes #94
Note to reviewers: this PR addresses the two requirements because it seams more coherent to use cloud token for authentication when the build service from GCK6 is used.