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

feat: Add ability to use a config file for the CLI tool #145

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

guilgaly
Copy link
Member

@guilgaly guilgaly commented Mar 4, 2025

Goal: avoid having to always specify all options on the command line, when many of them will always be the same for a given project.

Still in draft, because there is a discussion to have about the file format and file location/name.

  • Current proposal is a simple JSON file because that's widely used by JS tools (tsc, prettier, etc.) and easy to support. But it's different from the HOCON format we use for the package descriptor file (.gatling/package.conf by default) and for the the gatling.conf file.
  • Location: should this file be expected to be in the .gatling folder (like the package descriptor file)? Tools like tsc/prettier/etc. just look for their config files at the root of the project folder by default, but since we already have this .gatling folder, we should probably use it...
  • File name: any good ideas?
    • Currently .gatling/cli.json for testing, but I don't like it...
  • Should this even be a separate file or could this be merged into the package descriptor file? Not sure this is a good idea (different purpose, format shared with JVM tools...), but having a bunch of different config files is not ideal for the end user either...

@guilgaly guilgaly requested a review from notdryft March 4, 2025 13:55
@guilgaly guilgaly self-assigned this Mar 4, 2025
@guilgaly
Copy link
Member Author

guilgaly commented Mar 5, 2025

Another thing: as I did things here, when executing npx gatling <some command> --help, the config values from the config file appear as the default values in the help text. It was easier to do that way, but might be a bit confusing?

Base automatically changed from truststore-config to main March 5, 2025 08:26
Comment on lines +19 to +40
const configFileValidation = object({
// --gatling-home <value>
gatlingHome: string.optional(),
// --sources-folder <value>
sourcesFolder: string.optional(),
// --resources-folder <value>
resourcesFolder: string.optional(),
// --results-folder <value>
resultsFolder: string.optional(),
// --bundle-file <value>
bundleFile: string.optional(),
// --package-file <value>
packageFile: string.optional(),
// --memory <value>
memory: number.optional(),
// args [optionKey=optionValue...]
runParameters: dictionary(string, string).default({}),
// --control-plane-url <value>
controlPlaneUrl: string.optional(),
// --package-descriptor-filename <value>
packageDescriptorFilename: string.optional()
});
Copy link
Member

Choose a reason for hiding this comment

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

feels like yup (validation library that we use for frontend code)

Copy link
Member

Choose a reason for hiding this comment

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

Location

As you mentioned, I prefer to be in the .gatling directory.
I hate tools that force to be in root, that clutters the main repository.

I, personally, do support the dot-config initiative.

HOCON vs JSON

I personally go to HOCON because it allows for including other files, env var substitution, reusing part of configuration, etc, while accepting a plain JSON.

In same file or not?

If HOCON, I'll go in the same file. but different "main" key
current one is gatling.enterprise.package
we can imagine another key, suggestions:

  • gatling.enterprise.build
  • gatling.enterprise.cli
  • gatling.enterprise.local
  • gatling.enterprise.arguments
  • gatling.enterprise.js

Copy link
Member

Choose a reason for hiding this comment

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

I think the file is expected to be named package.conf by default, IMO it should be in another file in .gatling/. (HOCON allow file refs if users want it merged in a same file).

I agree on the key suggestion gatling.enterprise.cli

Copy link
Member

@tpetillot tpetillot left a comment

Choose a reason for hiding this comment

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


export type ConfigFile = typeof configFileValidation.T;

const configFileValidation = object({
Copy link
Member

Choose a reason for hiding this comment

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

Why would all these parameters except memory, runParameters and controlPlaneUrl be configurable by the end users? They shouldn't.

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

Successfully merging this pull request may close these issues.

4 participants