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

[PulsarAdmin] Functions to async #6580

Merged
merged 2 commits into from
Mar 23, 2020
Merged

[PulsarAdmin] Functions to async #6580

merged 2 commits into from
Mar 23, 2020

Conversation

yjshen
Copy link
Member

@yjshen yjshen commented Mar 21, 2020

No description provided.

@yjshen
Copy link
Member Author

yjshen commented Mar 21, 2020

/pulsarbot run-failure-checks

1 similar comment
@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

@codelipenghui codelipenghui added this to the 2.6.0 milestone Mar 21, 2020
@codelipenghui codelipenghui requested review from jiazhai and sijie March 21, 2020 09:03
@sijie sijie merged commit 75d43eb into apache:master Mar 23, 2020
codelipenghui added a commit that referenced this pull request Mar 26, 2020
### Motivation

There are some tests that failed in the master branch related to functions. For example https://github.com/apache/pulsar/blob/master/pulsar-broker/src/test/java/org/apache/pulsar/functions/worker/PulsarFunctionE2ESecurityTest.java#L630, the test checks the properties of the message are forwarded, but we don't call `functionConfig.setForwardSourceMessageProperty(true)`, the reason is #6318 introduce `forwardSourceMessageProperty ` to enable or disable message properties forward in functions. But the default value is false, this will break previous behavior. So this PR set it to true to keep forward compatible.

And getApiException method (https://github.com/apache/pulsar/blob/master/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/BaseResource.java#L235) is very confusing, this will cause the async function admin method that #6580 introduced can't complete when met exceptions. 

BTW,  I have no idea why #6318 and #6580 can pass the CI check. This test is there long ago.
@yjshen yjshen deleted the functions branch March 26, 2020 06:55
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
### Motivation

There are some tests that failed in the master branch related to functions. For example https://github.com/apache/pulsar/blob/master/pulsar-broker/src/test/java/org/apache/pulsar/functions/worker/PulsarFunctionE2ESecurityTest.java#L630, the test checks the properties of the message are forwarded, but we don't call `functionConfig.setForwardSourceMessageProperty(true)`, the reason is apache#6318 introduce `forwardSourceMessageProperty ` to enable or disable message properties forward in functions. But the default value is false, this will break previous behavior. So this PR set it to true to keep forward compatible.

And getApiException method (https://github.com/apache/pulsar/blob/master/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/BaseResource.java#L235) is very confusing, this will cause the async function admin method that apache#6580 introduced can't complete when met exceptions. 

BTW,  I have no idea why apache#6318 and apache#6580 can pass the CI check. This test is there long ago.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants