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

Change config directory name from "loadimpact" to "k6" #2508

Open
MattDodsonEnglish opened this issue Apr 28, 2022 · 7 comments · May be fixed by #4301
Open

Change config directory name from "loadimpact" to "k6" #2508

MattDodsonEnglish opened this issue Apr 28, 2022 · 7 comments · May be fixed by #4301
Assignees
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 feature

Comments

@MattDodsonEnglish
Copy link
Contributor

MattDodsonEnglish commented Apr 28, 2022

Feature Description

Looking in my .config , I happened to notice the loadimpact directory. Inside that directory, there was a k6 directory, which had a config.json file with my API key.

Can the loadimpact part be removed? Users looking for their configs will be much more likely to look for k6. Besides, not everyone will associate k6 with loadimpact. As time goes on, this association will become weaker and weaker, too, as the official business name is now k6.

Suggested Solution (optional)

$XDG_CONFIG_HOME/k6/config.json

For example,
~/.config/loadimpact/k6/config.json

~/.config/k6/config.json

Appropriate default directory locations might be different on Mac and Windows.

@mstoykov
Copy link
Contributor

This will likely also mean that we need code to:

  1. copy the old config in the place of the new one if it doesn't exist. But print warning if it does
  2. delete the old, so we don't print warnings.

Hopefully with the latest changes to the cmd package this will need less hacks to implement 🤞

@na-- na-- added this to the v0.39.0 milestone Apr 28, 2022
@na-- na-- added the evaluation needed proposal needs to be validated or tested before fully implementing it in k6 label Apr 28, 2022
@na--
Copy link
Member

na-- commented Apr 28, 2022

Yeah, not sure how we should handle old configs, but this should definitely be easier to implement and test nowadays 🤞

We should probably look into how other grafana projects handle this, if they have similar config files in .config/grafana, so we can move ours to .config/grafana/k6? 🤔

Personally, I am slightly against automatically moving .config/loadimpact during a k6 run command. Instead, I think we should:

  • support both .config/loadimpact and .config/grafana during k6 run, k6 cloud, etc., with .config/grafana being checked for and used first, and .config/loadimpact emitting a deprecation warning if it ends up being loaded
  • move the config files when k6 login is executed (and explain that in the deprecation warning ⬆️ )

@MattDodsonEnglish
Copy link
Contributor Author

I realized that the docs will need to be updated in a few places if this change happens:

@mstoykov mstoykov modified the milestones: v0.39.0, v0.40.0 Jun 15, 2022
@olegbespalov olegbespalov self-assigned this Jun 29, 2022
@na-- na-- removed this from the v0.40.0 milestone Jun 29, 2022
@olegbespalov olegbespalov removed their assignment Jul 5, 2022
@dgzlopes
Copy link
Member

dgzlopes commented May 3, 2023

I would go with .config/k6

As far as I know, in the Grafana world, this pattern doesn't exist. Also, it looks easier to find for k6 users.

@markjmeier
Copy link

+1 to I would go with .config/k6 for the same reasons Daniel mentions.

As for the deprecation warning, it would be good if it included instructions for updating (link to doc if not simple).

Sounds like for users using Grafana Cloud k6 - k6 login would be the proposed quick way to update. But how would this work for pure OSS users? What would they need to do?

@codebien
Copy link
Contributor

codebien commented May 4, 2023

Sounds like for users using Grafana Cloud k6 - k6 login would be the proposed quick way to update. But how would this work for pure OSS users? What would they need to do?

k6 login should be the unique command that writes the config, right? If k6 run and k6 cloud both support the two file paths as Ned suggested then we don't need to differentiate the cases.

When the user runs for the first time with the new version they will see the warning, so they can:

  • if they are still using the influxdb/cloud output then they can re-run k6 login and the config will be migrated otherwise
  • if they are not using the output anymore then they could drop the config file to not see the warning again

The unique edge case I've found is if they are using a custom config that they want to keep but they are not using the output and they wouldn't log in again. In this case, they should migrate the config manually.

Having the migration on k6 run and k6 cloud could be easier but I agree with Ned that having this migration on the main commands is a risk not justified by the benefits (with the assumption in mind that the most used case will be to just log in again).

@mstoykov
Copy link
Contributor

mstoykov commented May 9, 2023

Also of note is that we do print where the default config is being loaded from on help
(cut to the important parts

$ k6 --help
...
  -c, --config string       JSON config file (default "/home/mstoykov/.config/loadimpact/k6/config.json")
...

@andrewslotin andrewslotin changed the title Change config directory name from "loadimpact" to "k6" Change config directory name from "loadimpact" to "grafana" May 9, 2024
@andrewslotin andrewslotin changed the title Change config directory name from "loadimpact" to "grafana" Change config directory name from "loadimpact" to "k6" May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants