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

Fix the mix-up of command line options -L and -I (Fix #1413) #1418

Merged
merged 1 commit into from Apr 15, 2021

Conversation

dmikurube
Copy link
Member

@dmikurube dmikurube commented Apr 7, 2021

We have had a bug in command line parsing -- -L and -I have been mixed up. (#1413)

@dmikurube dmikurube added the bug Something isn't working label Apr 7, 2021
@dmikurube dmikurube added this to the v0.10.30 milestone Apr 7, 2021
@dmikurube dmikurube force-pushed the fix-mixup-of-cli-options-L-I branch from f848699 to 6af877e Compare April 7, 2021 08:19
@dmikurube
Copy link
Member Author

@hiroyuki-sato Trying to fix #1413. Could you have a quick look?

@hiroyuki-sato
Copy link
Contributor

@dmikurube Thanks! LGTM 👍

I checked those options behavior using embulk-parser-jsonpath.

git checkout -b topic/fix-mixup-of-cli-options-L-I upstream/fix-mixup-of-cli-options-L-I
./gradlew executableJar
./gradlew embulk-ruby:gemContents
cd path/to/embulk-parser-jsonpath
./gradlew gems
cd /path/to/embulk-parser-jsonpath

-L: option test.

/path/to/embulk/build/executable/embulk-0.10.30-SNAPSHOT.jar \
  -X embulk_home=/path/to/.embulk-dev \
  run  \
  -L build/gemContents \ # not contains lib
  -I path/to/embulk/embulk-ruby/build/gemContents/lib \
  example/conf.yml

-I: option test.

/path/to/embulk/build/executable/embulk-0.10.30-SNAPSHOT.jar \
  -X embulk_home=/path/to/.embulk-dev \
  run  \
  -I build/gemContents/lib \ # contains lib
  -I path/to/embulk/embulk-ruby/build/gemContents/lib \
  example/conf.yml
2021-04-07 20:24:39.469 +0900 [INFO] (main): embulk_home is set from command-line: /Users/user/.embulk-dev
2021-04-07 20:24:39.953 +0900 [INFO] (main): Started Embulk v0.10.30-SNAPSHOT
2021-04-07 20:24:42.654 +0900 [INFO] (0001:transaction): Gem's home and path are set by system configs "gem_home": "/Users/user/.embulk-dev/lib/gems", "gem_path": ""
2021-04-07 20:24:43.353 +0900 [INFO] (0001:transaction): Loaded plugin embulk/parser/jsonpath from a load path
2021-04-07 20:24:43.402 +0900 [INFO] (0001:transaction): Listing local files at directory 'example' filtering filename by prefix 'input'
2021-04-07 20:24:43.402 +0900 [INFO] (0001:transaction): "follow_symlinks" is set false. Note that symbolic links to directories are skipped.
2021-04-07 20:24:43.405 +0900 [INFO] (0001:transaction): Loading files [example/input.json, example/input3.json, example/input2.json]
2021-04-07 20:24:43.453 +0900 [INFO] (0001:transaction): Using local thread executor with max_threads=8 / output tasks 6 = input tasks 3 * 2
2021-04-07 20:24:43.464 +0900 [INFO] (0001:transaction): {done:  0 / 3, running: 0}
2021-04-07 20:24:43.536 +0900 [INFO] (0012:task-0000): JSONPath = $.results
2021-04-07 20:24:43.536 +0900 [INFO] (0014:task-0002): JSONPath = $.results
2021-04-07 20:24:43.537 +0900 [INFO] (0013:task-0001): JSONPath = $.results
2021-04-07 20:24:43.667 +0900 [INFO] (0001:transaction): {done:  2 / 3, running: 1}
2021-04-07 20:24:43.667 +0900 [INFO] (0001:transaction): {done:  3 / 3, running: 0}
2021-04-07 20:24:43.667 +0900 [INFO] (0001:transaction): {done:  3 / 3, running: 0}
2021-04-07 20:24:43.684 +0900 [INFO] (main): Committed.
2021-04-07 20:24:43.684 +0900 [INFO] (main): Next config diff: {"in":{"last_path":"example/input3.json"},"out":{}}

Base automatically changed from bump-up-to-v0.10.30 to master April 14, 2021 06:21
Copy link
Contributor

@shroman shroman left a comment

Choose a reason for hiding this comment

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

@hiroyuki-sato Thanks for verifying!

Copy link
Member

@himadieievsv himadieievsv left a comment

Choose a reason for hiding this comment

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

lgtm

@dmikurube
Copy link
Member Author

Thanks, all, for having a look! I'm going to release v0.10.30 with it soon.

@dmikurube dmikurube merged commit 699db9f into master Apr 15, 2021
@dmikurube dmikurube deleted the fix-mixup-of-cli-options-L-I branch April 15, 2021 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

None yet

4 participants