Update meltano select to print full error message on catalog discovery errors (and with other CLI commands if possible)

[astrojuanlu]

When a catalog discovery attempt fails, the error message does not give enough information about what happened:

$ meltano select tap-postgres --list --all
2022-06-10T14:46:22.487369Z [info     ] Environment 'dev' is active
Cannot list the selected attributes: Catalog discovery failed: command ['/home/juanlu/Projects/Orchest/Singer/test-replication-none/.meltano/extractors/tap-postgres/venv/bin/tap-postgres', '--config', '/home/juanlu/Projects/Orchest/Singer/test-replication-none/.meltano/run/tap-postgres/tap.c331e588-15d0-4ec6-aa62-90ec0c99c76f.config.json', '--discover'] returned 1

In fact, the intermediate .meltano/run/tap-postgres/tap.c331e588-15d0-4ec6-aa62-90ec0c99c76f.config.json config file gets automatically deleted, which makes debugging more cumbersome.

The current workaround is to:

1. Generate a config manually: meltano config tap-postgres > config.json
2. Rerun the command with the custom config, assuming it's the same as the temporary one generated on the fly: /home/juanlu/Projects/Orchest/Singer/test-replication-none/.meltano/extractors/tap-postgres/venv/bin/tap-postgres --config config.json --discover
3. See actual error

It would be nice if the "returned 1" message would also give the output of the command, for example:

$ /home/juanlu/Projects/Orchest/Singer/test-replication-none/.meltano/extractors/tap-postgres/venv/bin/tap-postgres --config config.json --discover
time=2022-06-10 16:49:52 name=tap_postgres level=CRITICAL message=Config is missing required keys: ['dbname']
Traceback (most recent call last):
  File "/home/juanlu/Projects/Orchest/Singer/test-replication-none/.meltano/extractors/tap-postgres/venv/bin/tap-postgres", line 8, in <module>
    sys.exit(main())
  File "/home/juanlu/Projects/Orchest/Singer/test-replication-none/.meltano/extractors/tap-postgres/venv/lib/python3.8/site-packages/tap_postgres/__init__.py", line 435, in main
    raise exc
  File "/home/juanlu/Projects/Orchest/Singer/test-replication-none/.meltano/extractors/tap-postgres/venv/lib/python3.8/site-packages/tap_postgres/__init__.py", line 432, in main
    main_impl()
  File "/home/juanlu/Projects/Orchest/Singer/test-replication-none/.meltano/extractors/tap-postgres/venv/lib/python3.8/site-packages/tap_postgres/__init__.py", line 393, in main_impl
    args = parse_args(REQUIRED_CONFIG_KEYS)
  File "/home/juanlu/Projects/Orchest/Singer/test-replication-none/.meltano/extractors/tap-postgres/venv/lib/python3.8/site-packages/tap_postgres/__init__.py", line 384, in parse_args
    utils.check_config(args.config, required_config_keys)
  File "/home/juanlu/Projects/Orchest/Singer/test-replication-none/.meltano/extractors/tap-postgres/venv/lib/python3.8/site-packages/singer/utils.py", line 190, in check_config
    raise Exception("Config is missing required keys: {}".format(missing_keys))
Exception: Config is missing required keys: ['dbname']

[DouweM]

@astrojuanlu Good call, I think I fixed this in meltano elt a while ago (https://gitlab.com/meltano/meltano/-/issues/2340), but we should definitely do the same in meltano select.

@tayloramurphy I think we should prioritize this as part of the "new user experience" goal.

[aaronsteers]

@cjohnhanson - Is it okay if I assign this to you? It might be a new part of the codebase for you to dive into, but would be a good exercise if so and valuable usability improvement either way. 😄

Also - you may have some prior art in the above linked issue which could be helpful.

[DouweM]

Related to https://gitlab.com/meltano/meltano/-/issues/2340.

[aaronsteers]

@tayloramurphy, @pandemicsyn, @cjohnhanson, @visch - This came up in office hours today related to other commands which might not be printing stderr for failed discovery operations.

@cjohnhanson - When you are back and ready to pick this up, you see about tackling discovery as part of all three of these code paths?:

• meltano select
• meltano invoke
• meltano run
• meltano elt (already done?)

From that office hours conversation, it sounded as though perhaps we've tackled elt but maybe not the others. Also, as is standard, it's totally acceptable if you decide it is best to solve in multiple smaller iterations; just wanted to make sure we're thinking about the full long-term scope as you begin the work.

Thanks!

Another proposal that came up in that conversation was to record all STDERR from discovery into a log file so that it is available for inspection even if --log-level=DEBUG is not set. I'm not sure what's best here but wanted to log that idea here in case it's helpful.