-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: Feature/kafkaoptions kwargs #4902
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
Open
zerafachris
wants to merge
9
commits into
feast-dev:master
Choose a base branch
from
zerafachris:feature/kafkaoptions_kwargs
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
29d13d0
modify spark kafka processor to remove repeated code
zerafachris e75d483
introduce kafka_settings to KafkaOptions
zerafachris 57e773f
pre-commit changes
zerafachris 555c614
recompile python lock files and build proto
zerafachris 1ef5d0a
include 'StreamFormat kafka_settings = 5;' to proto definition
zerafachris 897df59
resolve conflict
zerafachris 394fba0
resolve conflicts 2
zerafachris 0a1f724
Update DataSource.proto
zerafachris 3de66d5
Merge branch 'master' into feature/kafkaoptions_kwargs
zerafachris File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -41,11 +41,13 @@ def __init__( | |||||
message_format: StreamFormat, | ||||||
topic: str, | ||||||
watermark_delay_threshold: Optional[timedelta] = None, | ||||||
kafka_settings: Optional[Dict[str, str]] = None, | ||||||
): | ||||||
self.kafka_bootstrap_servers = kafka_bootstrap_servers | ||||||
self.message_format = message_format | ||||||
self.topic = topic | ||||||
self.watermark_delay_threshold = watermark_delay_threshold or None | ||||||
self.kafka_settings = kafka_settings or None | ||||||
|
||||||
@classmethod | ||||||
def from_proto(cls, kafka_options_proto: DataSourceProto.KafkaOptions): | ||||||
|
@@ -65,11 +67,15 @@ def from_proto(cls, kafka_options_proto: DataSourceProto.KafkaOptions): | |||||
if kafka_options_proto.watermark_delay_threshold.ToNanoseconds() == 0 | ||||||
else kafka_options_proto.watermark_delay_threshold.ToTimedelta() | ||||||
) | ||||||
|
||||||
kafka_options = cls( | ||||||
kafka_bootstrap_servers=kafka_options_proto.kafka_bootstrap_servers, | ||||||
message_format=StreamFormat.from_proto(kafka_options_proto.message_format), | ||||||
topic=kafka_options_proto.topic, | ||||||
watermark_delay_threshold=watermark_delay_threshold, | ||||||
kafka_settings=kafka_options_proto.kafka_settings | ||||||
if kafka_options_proto.HasField("kafka_settings") | ||||||
else None, | ||||||
) | ||||||
|
||||||
return kafka_options | ||||||
|
@@ -91,6 +97,7 @@ def to_proto(self) -> DataSourceProto.KafkaOptions: | |||||
message_format=self.message_format.to_proto(), | ||||||
topic=self.topic, | ||||||
watermark_delay_threshold=watermark_delay_threshold, | ||||||
kafka_settings=self.kafka_settings, | ||||||
) | ||||||
|
||||||
return kafka_options_proto | ||||||
|
@@ -361,6 +368,7 @@ def __init__( | |||||
owner: Optional[str] = "", | ||||||
batch_source: Optional[DataSource] = None, | ||||||
watermark_delay_threshold: Optional[timedelta] = None, | ||||||
kafka_settings: Optional[Dict[str, str]] = None | ||||||
): | ||||||
""" | ||||||
Creates a KafkaSource object. | ||||||
|
@@ -384,11 +392,12 @@ def __init__( | |||||
batch_source (optional): The datasource that acts as a batch source. | ||||||
watermark_delay_threshold (optional): The watermark delay threshold for stream data. | ||||||
Specifically how late stream data can arrive without being discarded. | ||||||
kafka_settings (optional): Optional kafka settings | ||||||
""" | ||||||
if bootstrap_servers: | ||||||
warnings.warn( | ||||||
( | ||||||
"The 'bootstrap_servers' parameter has been deprecated in favor of 'kafka_bootstrap_servers'. " | ||||||
"The 'bootstrap_servers' parameter has been deprecated in favour of 'kafka_bootstrap_servers'. " | ||||||
"Feast 0.25 and onwards will not support the 'bootstrap_servers' parameter." | ||||||
), | ||||||
DeprecationWarning, | ||||||
|
@@ -413,6 +422,7 @@ def __init__( | |||||
message_format=message_format, | ||||||
topic=topic, | ||||||
watermark_delay_threshold=watermark_delay_threshold, | ||||||
kafka_settings=kafka_settings | ||||||
) | ||||||
|
||||||
def __eq__(self, other): | ||||||
|
@@ -468,6 +478,8 @@ def from_proto(data_source: DataSourceProto): | |||||
if data_source.batch_source | ||||||
else None | ||||||
), | ||||||
kafka_settings=(DataSource.from_proto(data_source.kafka_options.kafka_settings) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect method call. kafka_settings should be used directly as a dictionary, not converted using DataSource.from_proto() which expects a DataSource protobuf message.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
if data_source.kafka_options.kafka_settings else None ) | ||||||
) | ||||||
|
||||||
def to_proto(self) -> DataSourceProto: | ||||||
|
This file contains hidden or 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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use American English spelling 'favor' instead of British English 'favour' for consistency with the codebase.
Copilot uses AI. Check for mistakes.