first cut at zinger rewrite as a kafka syncer #17
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mattnibs
reviewed
Sep 27, 2021
| } | ||
|
|
||
| type Credentials struct { | ||
| User string | ||
| Password string | ||
| } | ||
|
|
||
| const HostEnv = "ZED_LAKE_HOST" |
There was a problem hiding this comment.
Maybe use HostEnv from zed/cmd/zed/api/command.go?
There was a problem hiding this comment.
Yeah I copied this from there, but importing would bring in a bunch of stuff that isn't used. If we wanted to do this, we should move to this logic to zed/cli/hostflags
mattnibs
reviewed
Sep 27, 2021
| } | ||
|
|
||
| func (p *Producer) Run(ctx context.Context, reader zio.Reader) error { | ||
| fmt.Printf("producing messages to topic %q...\n", p.topic) |
There was a problem hiding this comment.
Should a library package be printing to stdout?
There was a problem hiding this comment.
Right now, the entirety of the zinger package is a CLI tool so I vote to fix this later.
nwt
reviewed
Sep 27, 2021
nwt
reviewed
Sep 27, 2021
fifo/from.go
Outdated
| return ncommit, nrec, nil | ||
| } | ||
|
|
||
| // AdjustOffsetsAndShape runs a local Zed program to adjust the kafka offset fields |
There was a problem hiding this comment.
Suggested change
| // AdjustOffsetsAndShape runs a local Zed program to adjust the kafka offset fields | |
| // AdjustOffsetsAndShape runs a local Zed program to adjust the Kafka offset fields |
nwt
reviewed
Sep 27, 2021
philrz
reviewed
Sep 27, 2021
README.md
Outdated
| > Note that `zinger sync from` does not currently support multiplexing multiple | ||
| > inbound topics, but support for this is straightforward and we will add it soon. | ||
| > | ||
| > We also need to adapt `sync from` so it updates te consumer commit offsets, |
There was a problem hiding this comment.
Suggested change
| > We also need to adapt `sync from` so it updates te consumer commit offsets, | |
| > We also need to adapt `sync from` so it updates the consumer commit offsets, |
nwt
reviewed
Sep 27, 2021
fifo/from.go
Outdated
| } | ||
| // This shouldn't happen since the consumer automatically adds | ||
| // this field. | ||
| return nil, fmt.Errorf("value read from Kafka topic missing kafka metadata field: %s", s) |
There was a problem hiding this comment.
Suggested change
| return nil, fmt.Errorf("value read from Kafka topic missing kafka metadata field: %s", s) | |
| return nil, fmt.Errorf("value read from Kafka topic missing 'kafka' metadata field: %s", s) |
fifo/from.go
Outdated
| // This should not happen. | ||
| err = fmt.Errorf("[ERR! %w]", err) | ||
| } | ||
| return nil, fmt.Errorf("kafka meta-data field is missing 'offset' field: %s", s) |
There was a problem hiding this comment.
Suggested change
| return nil, fmt.Errorf("kafka meta-data field is missing 'offset' field: %s", s) | |
| return nil, fmt.Errorf("'kafka' metadata field is missing 'offset' field: %s", s) |
nwt
reviewed
Sep 27, 2021
README.md
Outdated
| `zinger` is a connector between Kafka topics and Zed lakes. | ||
| It can run in either direction: syncing a Zed lake to a Kafka topic or | ||
| syncing a Kafka topic to a Zed lake. | ||
| zinger` is a connector between [Kafka](https://kafka.apache.org/) and |
There was a problem hiding this comment.
Suggested change
| zinger` is a connector between [Kafka](https://kafka.apache.org/) and | |
| `zinger` is a connector between [Kafka](https://kafka.apache.org/) and |
nwt
approved these changes
Sep 27, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.