meltano icon indicating copy to clipboard operation
meltano copied to clipboard

bug: `meltano state list` with pattern - no such option

Open pnadolny13 opened this issue 3 years ago • 5 comments

Meltano Version

2.3.0

Python Version

3.8

Bug scope

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

Operating System

Mac

Description

It looks like the --pattern argument thats in the docs https://docs.meltano.com/reference/command-line-interface#list isnt available on the CLI.

(meltano) Patricks-MBP:data pnadolny$ meltano --version
meltano, version 2.3.0

(meltano) Patricks-MBP:data pnadolny$ meltano state list --pattern '*tap-gitlab*'
2022-07-25T21:31:25.438941Z [info     ] Environment 'userdev' is active
Usage: meltano state list [OPTIONS] [PATTERN]
Try 'meltano state list --help' for help.

Error: No such option: --pattern

Code

No response

pnadolny13 avatar Jul 25 '22 21:07 pnadolny13

FYI @aaronsteers I'm adding this to next week's iteration to fix.

tayloramurphy avatar Jul 26 '22 15:07 tayloramurphy

@tayloramurphy - Before we start dev work on this, I just want to confirm the spec regarding how wildcards should be implemented.

Is it a correct expectation that we'll use the same parser that is used by meltano select syntax syntax?

The docs for meltano state list currently only mention * as wildcard, but the select rules are documented as:

Unix shell-style wildcards can be used in selection patterns to match multiple entities or attributes at once:

*: matches any sequence of characters ?: matches one character [abc]: matches either a, b, or c [!abc]: matches any character but a, b, or c

Sidebar: select docs don't give instructions on how to escape characters, although it would not be common to have any of these characters in a state ID, stream name, or property name, so perhaps escaping will not be necessary. (Mentioned characters are [*, !, ?, [, ]].)

Simplest solution is probably just to implement the same parser as in meltano select (refactoring into the utils library if needed for reuse?) and then for docs, we can just say something like:

- The `--pattern` option allows filtering returned state IDs by using * as a wildcard.
+ The `--pattern` option allows filtering returned state IDs by wildcards. Pattern matching follows [meltano select](https://docs.meltano.com/reference/command-line-interface#select) syntax.

aaronsteers avatar Aug 05 '22 19:08 aaronsteers

@kgpayne - Let me know (in accordance also with Taylor's response to the above), if you feel comfortable picking this up next week. Thanks!

I expect a weight between 2 and 4 and I for now I have placed a more conservative 4 for estimation.

I don't know how robust our implementation is currently, and my scoping takes into account that I believe the higher priority here should be code reuse (if at all possible), rather than trying to create from scratch a perfect/best pattern matching spec.

aaronsteers avatar Aug 05 '22 19:08 aaronsteers

Simplest solution is probably just to implement the same parser as in meltano select (refactoring into the utils library if needed for reuse?) and then for docs, we can just say something like:

@aaronsteers @kgpayne If it's actually simpler to use the more expressive matching patterns, then yeah I'm great with that. At a minimum I just want it to match what's in the docs, but if we get enhanced matching that's a nice bonus.

tayloramurphy avatar Aug 05 '22 20:08 tayloramurphy

Related:

  • https://github.com/meltano/meltano/discussions/6270

aaronsteers avatar Aug 09 '22 16:08 aaronsteers

@aaronsteers @tayloramurphy this ended up being really really simple - pattern matching was implemented with push-down to the database, but the CLI pattern was defined as an argument and not an option (as documented). It was a 1-line fix.

However this only gives us what is documented (i.e. use of *), rather than the more expressive options available in select. Given that the implementation of wildcards was done already, and that the select features are not readily reusable, I think it best to go with the minimum " I just want it to match what's in the docs" unless you feel strongly that more expressive pattern matching is needed at this stage 🙂

kgpayne avatar Aug 11 '22 10:08 kgpayne

@aaronsteers @tayloramurphy this ended up being really really simple - pattern matching was implemented with push-down to the database, but the CLI pattern was defined as an argument and not an option (as documented). It was a 1-line fix.

However this only gives us what is documented (i.e. use of *), rather than the more expressive options available in select. Given that the implementation of wildcards was done already, and that the select features are not readily reusable, I think it best to go with the minimum " I just want it to match what's in the docs" unless you feel strongly that more expressive pattern matching is needed at this stage 🙂

@kgpayne - Good call. And thanks for the quick turnaround.

As we discussed offline, there are advantages to keeping a simple * wildcard for now: specifically that this is more likely to be supported as a passthrough glob by state backends during their native list operations.

aaronsteers avatar Aug 11 '22 18:08 aaronsteers