Skip to content

[fix][cli] Pulsar shell: allow absolute config file - #18805

Merged
nicoloboschi merged 2 commits into
apache:masterfrom
nicoloboschi:shell-abs
Dec 13, 2022
Merged

[fix][cli] Pulsar shell: allow absolute config file#18805
nicoloboschi merged 2 commits into
apache:masterfrom
nicoloboschi:shell-abs

Conversation

@nicoloboschi

Copy link
Copy Markdown
Contributor

Motivation

When creating a config with the --file option, absolute paths are not allowed.

Modifications

  • Do not prepend the current dir if the passed file is absolute

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Dec 7, 2022
@nicoloboschi nicoloboschi changed the title [fix][cli] Pulsar shell: allows absolute config file [fix][cli] Pulsar shell: allow absolute config file Dec 7, 2022

@tisonkun tisonkun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since the baseDir is fixed in our case, I'd prefer:

static File resolveLocalFile(String input) {
  final File file = new File(input);
  if (!file.isAbsolute()) {
    return new File(LOCAL_FILES_BASE_DIR, input);
  }
  return file;
}

But it's a style preference, so not a blocker to merge this patch.

+1

@codecov-commenter

codecov-commenter commented Dec 12, 2022

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.41%. Comparing base (68ca60c) to head (5a38e1f).
⚠️ Report is 3181 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #18805      +/-   ##
============================================
- Coverage     50.05%   47.41%   -2.65%     
+ Complexity    11024    10581     -443     
============================================
  Files           703      703              
  Lines         68814    68814              
  Branches       7378     7378              
============================================
- Hits          34446    32629    -1817     
- Misses        30621    32511    +1890     
+ Partials       3747     3674      -73     
Flag Coverage Δ
unittests 47.41% <ø> (-2.65%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 127 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Technoboy- Technoboy- added this to the 2.12.0 milestone Dec 12, 2022
@nicoloboschi
nicoloboschi merged commit 1df65b9 into apache:master Dec 13, 2022
@nicoloboschi
nicoloboschi deleted the shell-abs branch December 13, 2022 08:32
nicoloboschi added a commit to datastax/pulsar that referenced this pull request Dec 13, 2022
Demogorgon314 pushed a commit to Demogorgon314/pulsar that referenced this pull request Dec 26, 2022
Demogorgon314 pushed a commit to Demogorgon314/pulsar that referenced this pull request Dec 29, 2022
lifepuzzlefun pushed a commit to lifepuzzlefun/pulsar that referenced this pull request Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants