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

add java8 date and time type to pulsar's primitive schemas #7874

Merged

Conversation

jianyun8023
Copy link
Contributor

Motivation

Compatible with flink 1.11 need to use java8 date api in pulsar's primitive schemas.

Modifications

Add Instant, LocalDate, LocalTime, LocalDateTime to pulsar's primitive schemas

Verifying this change

Add Instant, LocalDate, LocalTime, LocalDateTime types to the Schema type test

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

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

@jianyun8023 When you update the PulsarApi.proto, you need also to push the generated java file. To generate the new java file, you can run ./pulsar-common/generate_protobuf_docker.sh.

And please add unit test for the new schema type to ensure encoding and decoding are works well.

@jianyun8023
Copy link
Contributor Author

@jianyun8023 When you update the PulsarApi.proto, you need also to push the generated java file. To generate the new java file, you can run ./pulsar-common/generate_protobuf_docker.sh.

And please add unit test for the new schema type to ensure encoding and decoding are works well.

Ok thank you.

@jianyun8023
Copy link
Contributor Author

/pulsarbot run-failure-checks

@jiazhai
Copy link
Member

jiazhai commented Aug 22, 2020

@gaoran10 would you please also review this ?

@jianyun8023
Copy link
Contributor Author

/pulsarbot run-failure-checks

@sijie
Copy link
Member

sijie commented Aug 24, 2020

/pulsarbot run-failure-checks

@jianyun8023
Copy link
Contributor Author

/pulsarbot run-failure-checks

@jianyun8023
Copy link
Contributor Author

/pulsarbot run-failure-checks

1 similar comment
@jiazhai
Copy link
Member

jiazhai commented Aug 27, 2020

/pulsarbot run-failure-checks

@jiazhai
Copy link
Member

jiazhai commented Aug 27, 2020

@codelipenghui Would you please help review this again?

@sijie
Copy link
Member

sijie commented Aug 27, 2020

@codelipenghui can you review this again?

@codelipenghui codelipenghui merged commit e36458c into apache:master Aug 28, 2020
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
### Motivation

*Compatible with flink 1.11 need to use java8 date api in pulsar's primitive schemas.*

### Modifications

*Add Instant, LocalDate, LocalTime, LocalDateTime to pulsar's primitive schemas*

### Verifying this change

Add Instant, LocalDate, LocalTime, LocalDateTime types to the Schema type test
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
### Motivation

*Compatible with flink 1.11 need to use java8 date api in pulsar's primitive schemas.*

### Modifications

*Add Instant, LocalDate, LocalTime, LocalDateTime to pulsar's primitive schemas*

### Verifying this change

Add Instant, LocalDate, LocalTime, LocalDateTime types to the Schema type test
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
### Motivation

*Compatible with flink 1.11 need to use java8 date api in pulsar's primitive schemas.*

### Modifications

*Add Instant, LocalDate, LocalTime, LocalDateTime to pulsar's primitive schemas*

### Verifying this change

Add Instant, LocalDate, LocalTime, LocalDateTime types to the Schema type test
@jianyun8023 jianyun8023 deleted the add-java8-date-time-to-schema branch December 10, 2020 02:40
@jianyun8023
Copy link
Contributor Author

jianyun8023 commented Dec 10, 2020

@Huanli-Meng Some Date types have been added here, which needs to be added at https://pulsar.apache.org/docs/en/schema-understand/.
Please help me increase it.

@Huanli-Meng
Copy link
Contributor

@Huanli-Meng A new date type has been added here, which needs to be added at https://pulsar.apache.org/docs/en/schema-understand/.

@jianyun8023 , Got it. Will update the doc accordingly. BTW, it is for releases 2.7.0 and master, right?

@jianyun8023
Copy link
Contributor Author

@Huanli-Meng A new date type has been added here, which needs to be added at https://pulsar.apache.org/docs/en/schema-understand/.

@jianyun8023 , Got it. Will update the doc accordingly. BTW, it is for releases 2.7.0 and master, right?

Yes.

@Huanli-Meng
Copy link
Contributor

@Huanli-Meng A new date type has been added here, which needs to be added at https://pulsar.apache.org/docs/en/schema-understand/.

@jianyun8023 , Got it. Will update the doc accordingly. BTW, it is for releases 2.7.0 and master, right?

Yes.

OK. Thanks

jiazhai pushed a commit that referenced this pull request Dec 20, 2020
### Motivation

In PR #7874, Instant, LocalDate, LocalTime, LocalDateTime are added to Pulsar's primitive schemas codes. But doc is not updated accordingly. 

### Modifications

Update the Pulsar docs to support Instant, LocalDate, LocalTime, LocalDateTime for Pulsar's primitive schemas.

- Affected docs: Understand schema
     - sections: primitive type.
Affected releases: master and 2.7.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants