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/10/26 07:39:21 UTC
[GitHub] [pulsar] mingyifei opened a new pull request #12496: #12495
mingyifei opened a new pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496
<!--
### Contribution Checklist
- Name the pull request in the form "[Issue XYZ][component] Title of the pull request", where *XYZ* should be replaced by the actual issue number.
Skip *Issue XYZ* if there is no associated github issue for this pull request.
Skip *component* if you are unsure about which is the best component. E.g. `[docs] Fix typo in produce method`.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- Each pull request should address only one issue, not mix up code from multiple issues.
- Each commit in the pull request has a meaningful commit message
- Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below.
**(The sections below can be removed for hotfixes of typos)**
-->
*(If this PR fixes a github issue, please add `Fixes #<xyz>`.)*
Fixes #<xyz>
*(or if this PR is one task of a github issue, please add `Master Issue: #<xyz>` to link to the master issue.)*
Master Issue: #<xyz>
### Motivation
*Explain here the context, and why you're making that change. What is the problem you're trying to solve.*
### Modifications
*Describe the modifications you've done.*
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
*(Please pick either of the following options)*
This change is a trivial rework / code cleanup without any test coverage.
*(or)*
This change is already covered by existing tests, such as *(please describe tests)*.
*(or)*
This change added tests and can be verified as follows:
*(example:)*
- *Added integration tests for end-to-end deployment with large payloads (10MB)*
- *Extended integration test for recovery after broker failure*
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): (yes / no)
- The public API: (yes / no)
- The schema: (yes / no / don't know)
- The default values of configurations: (yes / no)
- The wire protocol: (yes / no)
- The rest endpoints: (yes / no)
- The admin cli options: (yes / no)
- Anything that affects deployment: (yes / no / don't know)
### Documentation
Check the box below and label this PR (if you have committer privilege).
Need to update docs?
- [ ] `doc-required`
(If you need help on updating docs, create a doc issue)
- [ ] `no-need-doc`
(Please explain why)
- [ ] `doc`
(If this PR contains doc changes)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-957037486
> > > Great doc and user can define the custom `Serialization` following your doc if they have special requirements. But I am wondering if changing the default setup would bring some compatibility issue in some corner case.
> >
> >
> > You can see if the configuration reference in the code will cause compatibility issues
>
> I'm referring the compatibility issue during upgrade for previous pulsar user, since you are changing the default behavior.
Understand that if the user upgrades the version and ignores this change point, it will cause unforeseen problems, right?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-951745484
> @mingyifei seems that you did not provide doc-related info. Does this PR need to update the doc?
> @mingyifei seems that you did not provide doc-related info. Does this PR need to update the doc?
Ok i will submit the document later
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-956191468
> Great doc and user can define the custom `Serialization` following your doc if they have special requirements. But I am wondering if changing the default setup would bring some compatibility issue in some corner case.
You can see if the configuration reference in the code will cause compatibility issues
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-957132999
> > > > Great doc and user can define the custom `Serialization` following your doc if they have special requirements. But I am wondering if changing the default setup would bring some compatibility issue in some corner case.
> > >
> > >
> > > You can see if the configuration reference in the code will cause compatibility issues
> >
> >
> > I'm referring the compatibility issue during upgrade for previous pulsar user, since you are changing the default behavior.
>
> Understand that if the user upgrades the version and ignores this change point, it will cause unforeseen problems, right?
Yep.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-956189410
> @mingyifei hi, dose field is null the attribute have any meaning? we also provide a way to bypass the default not allowed to be empty. Why should we modify it?
Perhaps you think that null attributes are meaningless, but not necessarily for business people. Even null is meaningful in business. Too many restrictions increase the limitations of the message. Wouldn't it be better to send arbitrary messages?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-958728624
> > > > > Great doc and user can define the custom `Serialization` following your doc if they have special requirements. But I am wondering if changing the default setup would bring some compatibility issue in some corner case.
> > > >
> > > >
> > > > You can see if the configuration reference in the code will cause compatibility issues
> > >
> > >
> > > I'm referring the compatibility issue during upgrade for previous pulsar user, since you are changing the default behavior.
> >
> >
> > Understand that if the user upgrades the version and ignores this change point, it will cause unforeseen problems, right?
>
> Yep.
How do you deal with similar pr?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei removed a comment on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei removed a comment on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-951745484
> @mingyifei seems that you did not provide doc-related info. Does this PR need to update the doc?
> @mingyifei seems that you did not provide doc-related info. Does this PR need to update the doc?
Ok i will submit the document later
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: #12495
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-951651657
> @mingyifei：感谢您的贡献。对于这个 PR，我们需要更新文档吗？ （[PR 模板包含有关 doc 的信息](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation)，这有助于其他人更多地了解更改。您能否在此和未来的 PR 描述中提供与 doc 相关的信息？谢谢）
> @mingyifei：感谢您的贡献。对于这个 PR，我们需要更新文档吗？ （[PR 模板包含有关 doc 的信息](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation)，这有助于其他人更多地了解更改。您能否在此和未来的 PR 描述中提供与 doc 相关的信息？谢谢）
> @mingyifei:Thanks for your contribution. For this PR, do we need to update docs? (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
Okay, I added it, do you think it's okay?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-958728043
> > > > > Great doc and user can define the custom `Serialization` following your doc if they have special requirements. But I am wondering if changing the default setup would bring some compatibility issue in some corner case.
> > > >
> > > >
> > > > You can see if the configuration reference in the code will cause compatibility issues
> > >
> > >
> > > I'm referring the compatibility issue during upgrade for previous pulsar user, since you are changing the default behavior.
> >
> >
> > Understand that if the user upgrades the version and ignores this change point, it will cause unforeseen problems, right?
>
> Yep.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #12496: #12495 Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-951730924
@mingyifei seems that you did not provide doc-related info. Does this PR needs to update doc?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-951766474
> @mingyifei seems that you did not provide doc-related info. Does this PR need to update the doc?
@Anonymitaet Document update completed
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei removed a comment on pull request #12496: #12495
Posted by GitBox <gi...@apache.org>.
mingyifei removed a comment on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-951651657
> @mingyifei：感谢您的贡献。对于这个 PR，我们需要更新文档吗？ （[PR 模板包含有关 doc 的信息](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation)，这有助于其他人更多地了解更改。您能否在此和未来的 PR 描述中提供与 doc 相关的信息？谢谢）
> @mingyifei：感谢您的贡献。对于这个 PR，我们需要更新文档吗？ （[PR 模板包含有关 doc 的信息](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation)，这有助于其他人更多地了解更改。您能否在此和未来的 PR 描述中提供与 doc 相关的信息？谢谢）
> @mingyifei:Thanks for your contribution. For this PR, do we need to update docs? (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
Okay, I added it, do you think it's okay?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-957037486
> > > Great doc and user can define the custom `Serialization` following your doc if they have special requirements. But I am wondering if changing the default setup would bring some compatibility issue in some corner case.
> >
> >
> > You can see if the configuration reference in the code will cause compatibility issues
>
> I'm referring the compatibility issue during upgrade for previous pulsar user, since you are changing the default behavior.
Understand that if the user upgrades the version and ignores this change point, it will cause unforeseen problems, right?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-956189089
> @mingyifei hi, dose field is null the attribute have any meaning? we also provide a way to bypass the default not allowed to be empty. Why should we modify it?
Perhaps you think that null attributes are meaningless, but not necessarily for business people. Even null is meaningful in business. Too many restrictions increase the limitations of the message. Wouldn't it be better to send arbitrary messages?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] congbobo184 commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
congbobo184 commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-954401652
@mingyifei hi, dose field is null the attribute have any meaning? we also provide a way to bypass the default not allowed to be empty. Why should we modify it?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: #12495
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-951651969
> @mingyifei:Thanks for your contribution. For this PR, do we need to update docs? (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
Okay, I added it, do you think it's okay?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-957029321
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei closed pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei closed pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-954356583
Great doc and user can define the custom `Serialization` following your doc if they have special requirements.
But I am wondering if changing the default setup would bring some compatibility issue in some corner case.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-954353569
@eolivelli @Anonymitaet Help review, thanks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei closed pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei closed pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei closed pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei closed pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet edited a comment on pull request #12496: #12495 Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
Anonymitaet edited a comment on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-951730924
@mingyifei seems that you did not provide doc-related info. Does this PR need to update the doc?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-958728043
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-957037486
> > > Great doc and user can define the custom `Serialization` following your doc if they have special requirements. But I am wondering if changing the default setup would bring some compatibility issue in some corner case.
> >
> >
> > You can see if the configuration reference in the code will cause compatibility issues
>
> I'm referring the compatibility issue during upgrade for previous pulsar user, since you are changing the default behavior.
Understand that if the user upgrades the version and ignores this change point, it will cause unforeseen problems, right?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-958728043
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei closed pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei closed pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-951745784
> @mingyifei seems that you did not provide doc-related info. Does this PR need to update the doc?
Ok i will submit the document later
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #12496: #12495
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-951646598
@mingyifei:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on a change in pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on a change in pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#discussion_r738897586
##########
File path: site2/docs/schema-understand.md
##########
@@ -410,6 +410,24 @@ You can define the `schemaDefinition` to generate a `struct` schema.
User user = consumer.receive().getValue();
```
+4. Use a custom `Serialization` method, such as json configuration method, the following is the default
+
+ ```java
+ ObjectMapper mapper = new ObjectMapper();
+ mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
+ mapper.disable(SerializationFeature.FAIL_ON_EMPTY_BEANS);
+ mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
+ JacksonJsonWriter<User> writer = new JacksonJsonWriter<>(mapper);
+ JacksonJsonReader<User> reader = new JacksonJsonReader<>(mapper, User.class);
+
+ SchemaDefinition<User> schemaDefinition = SchemaDefinition.<User>builder()
+ .withPojo(User.class)
+ .withSchemaWriter(writer)
+ .withSchemaReader(reader)
+ .build();
+ ```
+ You can define `SchemaWriter` and `SchemaReader` as singletons
Review comment:
@congbobo184 can you help review it from the technical perspective? Thanks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei removed a comment on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei removed a comment on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-956189089
> @mingyifei hi, dose field is null the attribute have any meaning? we also provide a way to bypass the default not allowed to be empty. Why should we modify it?
Perhaps you think that null attributes are meaningless, but not necessarily for business people. Even null is meaningful in business. Too many restrictions increase the limitations of the message. Wouldn't it be better to send arbitrary messages?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mingyifei commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-956190012
> > @mingyifei hi, dose field is null the attribute have any meaning? we also provide a way to bypass the default not allowed to be empty. Why should we modify it?
>
> Perhaps you think that null attributes are meaningless, but not necessarily for business people. Even null is meaningful in business. Too many restrictions increase the limitations of the message. Wouldn't it be better to send arbitrary messages?
@congbobo184
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-957029321
> > Great doc and user can define the custom `Serialization` following your doc if they have special requirements. But I am wondering if changing the default setup would bring some compatibility issue in some corner case.
>
> You can see if the configuration reference in the code will cause compatibility issues
I'm referring the compatibility issue during upgrade for previous pulsar user, since you are changing the default behavior.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #12496: [Issue #12495] Fix the serialization time attribute is not allowed to be empty by default
Posted by GitBox <gi...@apache.org>.
Jason918 commented on pull request #12496:
URL: https://github.com/apache/pulsar/pull/12496#issuecomment-957029321
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org