You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/01/05 01:45:08 UTC
[GitHub] [pulsar] flowchartsman opened a new pull request #9124: Issue #9123 Go Functions
flowchartsman opened a new pull request #9124:
URL: https://github.com/apache/pulsar/pull/9124
Fixes #9123
### Modifications
Check for nil ProducerSpec first
### Verifying this change
as in #9123
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie merged pull request #9124: Issue #9123 Go Functions
Posted by GitBox <gi...@apache.org>.
sijie merged pull request #9124:
URL: https://github.com/apache/pulsar/pull/9124
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet commented on pull request #9124: Issue #9123 Go Functions
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #9124:
URL: https://github.com/apache/pulsar/pull/9124#issuecomment-755822227
@flowchartsman I think this PR is great and LGTM since it add nil check for pointer, but we should work on another PR to make BatchBuilder exposed to user properly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet commented on pull request #9124: Issue #9123 Go Functions
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #9124:
URL: https://github.com/apache/pulsar/pull/9124#issuecomment-755329852
@flowchartsman @sijie I have some findings related to #8761.
Currently go functions get `KeyBasedBatchBuilder` support with #8761, but the `batchBuilderType` is not exposed as an individual argument in `pulsar-function-go/conf/conf.go` https://github.com/apache/pulsar/blob/master/pulsar-function-go/conf/conf.go, so user cannot pass `batchBuilderType` to function, and `batchBuilder` is not usable. Additional, `ProducerSpec` from `SinkSpec` are not initialized properly here: https://github.com/apache/pulsar/blob/master/pulsar-function-go/pf/instanceConf.go#L87-L90, and this will lead to #9123, user will get nil exception when access to `instanceConf.funcDetails.Sink.ProducerSpec`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] flowchartsman commented on pull request #9124: Issue #9123 Go Functions
Posted by GitBox <gi...@apache.org>.
flowchartsman commented on pull request #9124:
URL: https://github.com/apache/pulsar/pull/9124#issuecomment-756478971
@sijie should be rebased now, I believe
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #9124: Issue #9123 Go Functions
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #9124:
URL: https://github.com/apache/pulsar/pull/9124#issuecomment-756540270
@flowchartsman Thank you!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #9124: Issue #9123 Go Functions
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #9124:
URL: https://github.com/apache/pulsar/pull/9124#issuecomment-756540358
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] flowchartsman edited a comment on pull request #9124: Issue #9123 Go Functions
Posted by GitBox <gi...@apache.org>.
flowchartsman edited a comment on pull request #9124:
URL: https://github.com/apache/pulsar/pull/9124#issuecomment-755437715
@freeznet, it seems like what you're saying is that my fix is a band-aid at a lower level, and that the other changes would fix this the right way, is that correct? If so, I think I should close this PR and we should make a new one that correctly fixes all of the issues you mention.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #9124: Issue #9123 Go Functions
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #9124:
URL: https://github.com/apache/pulsar/pull/9124#issuecomment-755228703
@freeznet Can you review this change?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet edited a comment on pull request #9124: Issue #9123 Go Functions
Posted by GitBox <gi...@apache.org>.
freeznet edited a comment on pull request #9124:
URL: https://github.com/apache/pulsar/pull/9124#issuecomment-755822227
@flowchartsman I think this PR is great and LGTM since it add nil check for pointer, and the nil check is always welcome. But as you said, we should work on another PR to make BatchBuilder exposed to user properly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie merged pull request #9124: Issue #9123 Go Functions
Posted by GitBox <gi...@apache.org>.
sijie merged pull request #9124:
URL: https://github.com/apache/pulsar/pull/9124
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #9124: Issue #9123 Go Functions
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #9124:
URL: https://github.com/apache/pulsar/pull/9124#issuecomment-755228547
@flowchartsman Can you rebase to the latest master to include recent changes that fix the broken master?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] flowchartsman commented on pull request #9124: Issue #9123 Go Functions
Posted by GitBox <gi...@apache.org>.
flowchartsman commented on pull request #9124:
URL: https://github.com/apache/pulsar/pull/9124#issuecomment-755437715
@freeznet, it seems like what your saying is that my fix is a band-aid at a lower level, and that the other changes would fix this the right way, is that correct? If so, I think I should close this PR and we should make a new one that correctly fixes all of the issues you mention.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org