Skip to content
This repository was archived by the owner on Mar 3, 2026. It is now read-only.

Update Streaming Avro Record Output#433

Merged
kevinjnguyen merged 1 commit intomainfrom
debug/streaming
Jun 17, 2023
Merged

Update Streaming Avro Record Output#433
kevinjnguyen merged 1 commit intomainfrom
debug/streaming

Conversation

@kevinjnguyen
Copy link
Contributor

@kevinjnguyen kevinjnguyen commented Jun 14, 2023

PR with the progress made towards debugging the streaming implementation.

Changes:

  • Reverted the revert for Publish Time. The implementation was correct, just missing one more step.
  • Added the missing step for publish time. When reading the stream, a flag is passed to consider adding the publish time.
    • In the execution phase, no publish time (false).
    • In the preparation phase, publish time (true).

@kevinjnguyen kevinjnguyen requested a review from epinzur June 14, 2023 03:12
@cla-bot cla-bot bot added the cla-signed Set when all authors of a PR have signed our CLA label Jun 14, 2023
@kevinjnguyen kevinjnguyen changed the title Debug Streaming Implementation (draft) Debug Streaming Implementation Jun 16, 2023
@kevinjnguyen kevinjnguyen changed the title Debug Streaming Implementation Update Streaming Avro Record Output Jun 16, 2023
@kevinjnguyen kevinjnguyen enabled auto-merge June 16, 2023 15:31
Copy link
Collaborator

@bjchambers bjchambers left a comment

Choose a reason for hiding this comment

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

Seems fine for getting things moving.

Ideally, we'd either introduce an enum for the boolean or we'd simplify things so we didn't need to pass it (possibly deciding based on the presence of the _publish_time field in the schema that we need to add it). If we did the enum, it would let us write something PublishTime::Add or PublishTime::None rather than true and false, which would make it clearer what the booleans mean, especially in the cases of the functions that take multiple booleans.

@kevinjnguyen kevinjnguyen added this pull request to the merge queue Jun 17, 2023
Merged via the queue into main with commit cb631e4 Jun 17, 2023
@kevinjnguyen kevinjnguyen deleted the debug/streaming branch June 17, 2023 14:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed Set when all authors of a PR have signed our CLA python sparrow wren

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants