Skip to content
This repository has been archived by the owner on Dec 14, 2022. It is now read-only.

catalogTable support default properties #247

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

catalogTable support default properties #247

wants to merge 3 commits into from

Conversation

hnail
Copy link
Contributor

@hnail hnail commented Jan 25, 2021

Motivation

  1. make catalog format optional.
  2. PulsarCatalog.getTable() support table default properties by pulsar SchemaRegistry.

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): (no)
  • The public API: (no)
  • The schema: (no )
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (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 (no)

@hnail hnail requested review from jianyun8023, reswqa and a team as code owners January 25, 2021 23:31
@jiazhai
Copy link
Contributor

jiazhai commented Jan 26, 2021

@jianyun8023 Would you please help review this?

reswqa
reswqa approved these changes Jan 26, 2021
View reviewed changes
Copy link
Contributor

@reswqa reswqa left a comment

Choose a reason for hiding this comment

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

LGTM, and can you help with some additional test cases?

jianyun8023
jianyun8023 reviewed Jan 26, 2021
View reviewed changes
Copy link
Contributor

@jianyun8023 jianyun8023 left a comment

Choose a reason for hiding this comment

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

I look great.

@syhily syhily self-assigned this Nov 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jianyun8023 jianyun8023 jianyun8023 left review comments

@reswqa reswqa reswqa approved these changes

At least 1 approving review is required to merge this pull request.

Assignees

@syhily syhily

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@hnail @jiazhai @jianyun8023 @reswqa @syhily