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

feature: Support passing contents of file to meltano config <plugin> set <setting> via stdin #8177

Closed
edgarrmondragon opened this issue Sep 22, 2023 · 7 comments · Fixed by #8228

Comments

@edgarrmondragon
Copy link
Collaborator

edgarrmondragon commented Sep 22, 2023

Feature scope

CLI (options, error messages, logging, etc.)

Description

Current situation

There's a few ways to set a config value at the moment:

# dot-separated setting name
meltano config meltano set cli.log_level info

# space-separated setting name
meltano config meltano set cli log_level info

# interactive
meltano config meltano set --interactive

Proposal

Allow users to set a config value from a file. This is particularly useful when the value is a multi-line string or some other value that's stored in a file somewhere.

The GitHub CLI supports this:

$ gh secret set       
must pass name argument

Usage:  gh secret set <secret-name> [flags]

Flags:
  -a, --app string           Set the application for a secret: {actions|codespaces|dependabot}
  -b, --body string          The value for the secret (reads from standard input if not specified)
  -e, --env environment      Set deployment environment secret
  -f, --env-file file        Load secret names and values from a dotenv-formatted file
      --no-store             Print the encrypted, base64-encoded value instead of storing it on Github
  -o, --org organization     Set organization secret
  -r, --repos repositories   List of repositories that can access an organization or user secret
  -u, --user                 Set a secret for your user
  -v, --visibility string    Set visibility for an organization secret: {all|private|selected} (default "private")

$ gh secret set VERY_SECRET                    
? Paste your secret

$ gh secret set VERY_SECRET < private_key.txt 
✓ Set Actions secret VERY_SECRET for org/repo

Notice how there's 3 ways of supplying input:

  1. Interactively by omitting a value.
  2. Non-interactively by supplying a value to the --body option.
  3. Non-interactively by omitting a value and reading from stdin.

The difficulty in implementing in our case is that we can't omit the value in non-interactive input because we accept multi-valued setting names:

# Meltano would think the setting is 'path.to.nested' with value
# 'setting' rather than 'path.to.nested.setting' with a value read
# from stdin
meltano config meltano set path to nested setting

One way to support stdin would be to use a special value to tell Meltano to read from stdin, perhaps -:

$ meltano config my-plugin set private_key - < private_key.txt
Extractor 'my-plugin' setting 'private_key' was set in `.env`: 'dummy\n'

$ cat private_key.txt | meltano config my-plugin set private_key -
Extractor 'my-plugin' setting 'private_key' was set in `.env`: 'dummy2\n'
@XshubhamX
Copy link
Contributor

Seems interesting, @edgarrmondragon can I work on this.

@edgarrmondragon
Copy link
Collaborator Author

Go for it @XshubhamX

@XshubhamX
Copy link
Contributor

Instead of making the flow complex it more complex, why can’t we just add this support inside the interactive flow.

Current User Journey

  1. User wants to provide stdin input, they use the --interactive flag as follows.
  2. Run this command meltano config meltano set --interactive cli log_level.
  3. They get a prompt to provide input and the flow ends after they do provide the input.

Now we want to allow user to provide data which includes complex big strings with line breaks. Now if we look closely this is also a way to provide stdin non-interactively.

So the way user provide the value of the variable remains same in both the cases which is stdin and whether they provide it interactively or non-interactively are 2 different cases below this. Something like this.

- Stdin
- Interactively
- Non-interactively

Suggestion

  1. Rename the --interactive flag to --stdin.
  2. So now when the user will run meltano config meltano set --stdin cli log_level.
  3. We’ll first check if the input is already provided by a file, which in this case would be false, so we’ll fall back to the already existing flow to acces input interactively. Something like what git does with the below mentioned command
$ gh secret set VERY_SECRET                    
? Paste your secret
  1. But if user provide the input through a file in the same command like this meltano config meltano set --stdin cli log_level < value.txt.
  2. Then we’ll read the input from the file and set the value of the variable, like what git does with the below mentioned command.
$ gh secret set VERY_SECRET < private_key.txt 
✓ Set Actions secret VERY_SECRET for org/repo

Code Changes. Set Config Function

    if interactive (Should be replaced with stdin):
        if sys.stdin.isatty():
            # If input is not provided from a file run the interactive flow
            interaction.configure_all()
        else:
            setting_name += (value,)
            value = click.get_text_stream("stdin").read()
            
            interaction.set_value(setting_name=setting_name,
                        value=value, store=store)
    else:
        interaction.set_value(setting_name=setting_name,
                              value=value, store=store)

Thoughts @edgarrmondragon

@edgarrmondragon
Copy link
Collaborator Author

Rename the --interactive flag to --stdin.

This would be a breaking change, so I would try to avoid.

I also can't see how this would solve this problem with how Meltano currently parses nested settings:

The difficulty in implementing in our case is that we can't omit the value in non-interactive input because we accept multi-valued setting names:

# Meltano would think the setting is 'path.to.nested' with value
# 'setting' rather than 'path.to.nested.setting' with a value read
# from stdin
meltano config meltano set path to nested setting

@XshubhamX
Copy link
Contributor

@edgarrmondragon As explained above in this case we'll first read the standard input and update the settings and value according to that.
like this

else:
    setting_name += (value,)
    value = click.get_text_stream("stdin").read()
    
    interaction.set_value(setting_name=setting_name,
                value=value, store=store)

@edgarrmondragon
Copy link
Collaborator Author

@edgarrmondragon As explained above in this case we'll first read the standard input and update the settings and value according to that. like this

else:
    setting_name += (value,)
    value = click.get_text_stream("stdin").read()
    
    interaction.set_value(setting_name=setting_name,
                value=value, store=store)

@XshubhamX Ok, that might work but a few cases that are worth testing:

  1. Top-level setting: meltano config set ... <setting_name>
  2. Dot-delimited nested setting: meltano config set ... <nested>.<setting>
  3. Multi-valued setting name input: meltano config set ... <nested> <setting>

@dresslife-shbh
Copy link

@edgarrmondragon Will check this and also enhance the tests for setting config.

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

Successfully merging a pull request may close this issue.

3 participants