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

[dev-utils/run] expose unexpected flags as more than just names #54080

Merged
merged 1 commit into from
Jan 7, 2020

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Jan 6, 2020

The @kbn/dev-utils run() helper automatically validates that all passed flags are expected, but when the flags.allowUnexpected option is set to true then the unexpected flags are passed to the run function as flags.unexpected. This was designed for the dev cli to use so it could forward the flags to the Kibana server, but was implemented in a somewhat useless way, only forwarding the names of the flags and not their values. Even supplying the parsed values is somewhat useless for our case as we'd have to convert them back into their unparsed representation before passing them to the Kibana server.

To fix this I've updated flags.unexpected to include the full flag and value from the argv array with one exception, shortcut flags which are defined as a single flag like -abcd are "exploded" as -a, -b, etc. and only the unexpected ones are passed. (see test case for an example, -x is expected but z and y are not).

@spalger spalger force-pushed the fix/dev-run-unexpected-argv branch 3 times, most recently from 336a274 to 6d0569d Compare January 7, 2020 01:44
@spalger spalger force-pushed the fix/dev-run-unexpected-argv branch from 6d0569d to 1639dc4 Compare January 7, 2020 01:46
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

  • 💔 Build #18275 failed 336a274e5fc38547503793c3428c5647f980a775
  • 💔 Build #18263 failed d2d7766d9f047c2c9b183d89a6bb9a5a87b3e5e4
  • 💔 Build #18260 failed 72291534b6218ceacb539c9ec9f0aeb13483dfae

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@spalger spalger marked this pull request as ready for review January 7, 2020 04:15
@spalger spalger requested a review from a team as a code owner January 7, 2020 04:15
@spalger spalger added release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v7.6.0 v8.0.0 labels Jan 7, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

Copy link
Member

@mistic mistic left a comment

Choose a reason for hiding this comment

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

That LGTM. I also agree with the design choices made here!

@spalger
Copy link
Contributor Author

spalger commented Jan 7, 2020

7.x/7.6: 8f8d083

@spalger spalger deleted the fix/dev-run-unexpected-argv branch January 7, 2020 21:52
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 8, 2020
* master: (55 commits)
  [ui/public/utils] Copy rarely used items to where they are consumed (elastic#53819)
  set AppArch team as an owner of the search endpoints (elastic#54131)
  Don't expose Elasticsearch client as Observable (elastic#53824)
  [SIEM] Cleanup unnecessary use of enzyme-to-json (elastic#53980)
  fix ui exports doc (elastic#54138)
  change markdown element title (elastic#54194)
  [Logs UI] Refactor log position to hooks (elastic#53540)
  [SIEM] Implement NP Plugin Setup (elastic#54030)
  [DOCS] Updates ML links (elastic#53613)
  sort renovate packages in config
  Spaces - fix flakey api tests (elastic#54154)
  Remove dependency that was causing effect to re-execute infinitely. (elastic#54160)
  [dev/run] expose unexpected flags as more than just names (elastic#54080)
  [DOCS] Moves index pattern doc to Discover (elastic#53347)
  [SIEM] Cleanup React imports (elastic#53981)
  Update eslint related packages (elastic#54107)
  [Uptime] Added date range filter into expanded list query (elastic#52609)
  [SIEM] Add react/display-name eslint rule (elastic#53107)
  [SIEM] Enable eslint prefer-template rule (elastic#53983)
  Elasticsearch snapshots automation (elastic#53706)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v7.6.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants