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

feat(kafka): support confluent references #3862

Merged
merged 3 commits into from Jan 17, 2022

Conversation

anshbansal
Copy link
Collaborator

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable)

@anshbansal anshbansal closed this Jan 10, 2022
@anshbansal anshbansal reopened this Jan 10, 2022
@github-actions
Copy link

github-actions bot commented Jan 10, 2022

Unit Test Results (build & test)

  45 files  ±0    45 suites  ±0   16m 38s ⏱️ + 4m 13s
502 tests ±0  450 ✔️ ±0  52 💤 ±0  0 ±0 

Results for commit 47eb14b. ± Comparison against base commit bb0943f.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Jan 10, 2022

Unit Test Results (metadata ingestion)

       5 files  ±    0         5 suites  ±0   32m 15s ⏱️ - 3m 44s
   274 tests +  30     274 ✔️ +  30    0 💤 ±0  0 ±0 
1 264 runs  +144  1 236 ✔️ +142  28 💤 +2  0 ±0 

Results for commit 47eb14b. ± Comparison against base commit bb0943f.

♻️ This comment has been updated with latest results.

@anshbansal anshbansal closed this Jan 10, 2022
@anshbansal anshbansal reopened this Jan 10, 2022
schema_seen[ref_subject] = tmp_schema
logger.debug(f"ref for {ref_subject} is {tmp_schema.schema.schema_str}")
schema_str = schema_str.replace(
f'"{ref_name}"',
Copy link
Contributor

Choose a reason for hiding this comment

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

Do the reference names always appear in double quotes? Can they be single quoted? What if this string appears in documentation? Does confluent_kafka provider some a parser for us to more deterministically replace the reference schemas names with their schema definitions to generate a well-formed AVRO schema?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Avro has to be valid JSON so this should be in double quotes always. confluent_kafka does not provide any such method.

@shirshanka
Copy link
Contributor

@anshbansal: could you add a unit test for this with a checked in sample schema repo with references?

@anshbansal
Copy link
Collaborator Author

@shirshanka Unit test added. Can this be merged now?

Copy link
Contributor

@shirshanka shirshanka left a comment

Choose a reason for hiding this comment

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

LGTM

@shirshanka shirshanka merged commit 400e0fe into datahub-project:master Jan 17, 2022
@anshbansal anshbansal deleted the support_confluent_ref branch January 18, 2022 04:49
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.

None yet

3 participants