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.

Sign up for GitHub

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

Jump to bottom

[fix][admin] Fix deprecated check #22653

Merged
merged 2 commits into from
May 8, 2024
Merged

[fix][admin] Fix deprecated check #22653

merged 2 commits into from
May 8, 2024

Conversation

nodece
Copy link
Member

@nodece nodece commented May 6, 2024

Motivation

After migrating to picocli, which does not allow passing unknown options:

bin/pulsar-admin sinks localrun --archive $PWD/connectors/pulsar-io-elastic-search-3.3.0-SNAPSHOT.nar --tenant public --namespace default --name elasticsearch-test-sink --sink-config-file $PWD/es-sink.yml --inputs elasticsearch_test
Exception in thread "main" picocli.CommandLine$UnmatchedArgumentException: Unknown options: '--deprecatedBrokerServiceUrl', 'pulsar://localhost:6650/'

The previous version had a filtering mechanism, but the implementation was incorrect.

Modifications

  • Fix deprecated check

Documentation

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

@nodece
[fix][admin] Fix deprecated check
8cb3037 
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 6, 2024
lhotari
lhotari reviewed May 6, 2024
View reviewed changes
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

Any test case?

@nodece
Add test
6ed5ff9 
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@nodece nodece self-assigned this May 6, 2024
@nodece
Copy link
Member Author

nodece commented May 6, 2024

@lhotari Done.

@nodece nodece requested a review from lhotari May 6, 2024 14:54
@lhotari lhotari merged commit 188355b into apache:master May 8, 2024
53 of 55 checks passed
@lhotari lhotari mentioned this pull request May 13, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lhotari lhotari lhotari approved these changes

Assignees

@nodece nodece

Labels
doc-not-needed Your PR changes do not impact docs ready-to-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nodece @lhotari