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

[pulsar-admin] Allow setting --forward-source-message-property to false when updating a pulsar function #12128

Merged
merged 1 commit into from
Oct 14, 2021
Merged

[pulsar-admin] Allow setting --forward-source-message-property to false when updating a pulsar function #12128

merged 1 commit into from
Oct 14, 2021

Conversation

yuruguo
Copy link
Contributor

@yuruguo yuruguo commented Sep 22, 2021

Motivation

The value of option --forward-source-message-property is true by default, but it cannot be set to false, which means that we cannot prevent the input message's properties from being forwarded to the output topic.

The problem is that JCommander doesn't expect arguments to boolean parameters by default.
This is explained in the JCommander manual and issue comment cbeust/jcommander#129 (comment) .

Details are similar to [pulsar-perf] Make it possible to disable poolMessages

Modifications

  • add arity = 1 to the JCommander Parameter annotation.
    JCommander requires specifying arity = 1 to boolean parameters that have a default value of true.

Documentation

  • no-need-doc

@@ -271,7 +271,7 @@ void processArguments() throws Exception {
protected Boolean retainKeyOrdering;
@Parameter(names = "--batch-builder", description = "BatcherBuilder provides two types of batch construction methods, DEFAULT and KEY_BASED. The default value is: DEFAULT")
protected String batchBuilder;
@Parameter(names = "--forward-source-message-property", description = "Forwarding input message's properties to output topic when processing")
@Parameter(names = "--forward-source-message-property", description = "Forwarding input message's properties to output topic when processing", arity = 1)
Copy link
Contributor

@MarvinCai MarvinCai Sep 22, 2021

Choose a reason for hiding this comment

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

can we update the doc to reflect the change, like to disable it use --forward-source-message-property false?
as user usually won't expect to provide parameter for a flag
also please rebase on latest master once this pr is merged, current CI is broken

Copy link
Contributor Author

@yuruguo yuruguo Sep 22, 2021

Choose a reason for hiding this comment

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

done, PTAL @MarvinCai

@Anonymitaet Anonymitaet added the doc-not-needed Your PR changes do not impact docs label Sep 23, 2021
@codelipenghui codelipenghui merged commit e316a53 into apache:master Oct 14, 2021
zeo1995 pushed a commit to zeo1995/pulsar that referenced this pull request Oct 14, 2021
* up/master: (26 commits)
  [pulsar-admin] Allow setting --forward-source-message-property to false when updating a pulsar function (apache#12128)
  [website][upgrade]feat: docs migration - Development (apache#12320)
  Update delete inactive topic configuration documentation (apache#12350)
  [PIP 95][Issue 12040][broker] Multiple bind addresses for Pulsar protocol (apache#12056)
  Added Debezium Source for MS SQL Server (apache#12256)
  Fix: flaky oracle tests (apache#12306)
  [C++] Use URL encoded content type for OAuth 2.0 authentication (apache#12341)
  [C++] Handle OAuth 2.0 exceptional cases gracefully (apache#12335)
  feat(cli): add restart command to pulsar-daemon (apache#12279)
  [client-tools] Remove redundant initial value (apache#12296)
  Make AuthenticationTokenTest to run on windows (apache#12329)
  [offload] fix FileSystemManagedLedgerOffloader can not cleanup outdated ledger data (apache#12309)
  [Doc]--Update contents for Pulsar adaptor for Apache Spark (apache#12338)
  [PIP 95][Issue 12040][broker] Improved multi-listener in standalone mode (apache#12066)
  [website][upgrade]feat: docs migration - Cookbooks (apache#12319)
  [testclient] Make --payload-file take effect in PerformanceClient (apache#12187)
  [website][upgrade]feat: docs migration - adaptor (apache#12318)
  [pulsar-client] Add partition-change api for producer/consumer interceptors (apache#12287)
  [Transaction]Fix lowWaterMark of TopicTransactionBuffer (apache#12312)
  [pulsar-admin] New option takes precedence over deprecated option (apache#12260)
  ...

# Conflicts:
#	site2/website-next/docusaurus.config.js
#	site2/website-next/versions.json
zeo1995 pushed a commit to zeo1995/pulsar that referenced this pull request Oct 14, 2021
* up/master: (37 commits)
  re-enabling integration tests for Sinks (apache#12307)
  [PIP 95][Issue 12040][web] Topic lookup with listener header (apache#12072)
  Fix the master CI broken with update dispatch rate block issue (apache#12360)
  Fix message being ignored when the non-persistent topic reader reconnect. (apache#12348)
  Fix log format. (apache#12346)
  [website][upgrade]feat: docs migration - version-2.7.2 Concepts and Architecture (apache#12354)
  [website][upgrade] feat: full docs migration for version 2.8.0 (apache#12359)
  [website][upgrade]feat: dynamic replace version info before build (apache#12337)
  Fix flaky tests: ElasticSearchClientTests (apache#12347)
  Use asyncCloseCursorLedger to replace cursorLedger.asyncClose method in the ManagedCursorImpl.VoidCallback#operationComplete (apache#12113)
  fix-npe-ZkBookieRackAffinityMapping (apache#11947)
  [pulsar-admin] Allow setting --forward-source-message-property to false when updating a pulsar function (apache#12128)
  [website][upgrade]feat: docs migration - Development (apache#12320)
  Update delete inactive topic configuration documentation (apache#12350)
  [PIP 95][Issue 12040][broker] Multiple bind addresses for Pulsar protocol (apache#12056)
  Added Debezium Source for MS SQL Server (apache#12256)
  Fix: flaky oracle tests (apache#12306)
  [C++] Use URL encoded content type for OAuth 2.0 authentication (apache#12341)
  [C++] Handle OAuth 2.0 exceptional cases gracefully (apache#12335)
  feat(cli): add restart command to pulsar-daemon (apache#12279)
  ...

# Conflicts:
#	site2/website-next/docusaurus.config.js
#	site2/website-next/versioned_sidebars/version-2.7.2-sidebars.json
#	site2/website-next/versions.json
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/admin doc-not-needed Your PR changes do not impact docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants