Skip to content

Build fix for Kafka #1079

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 8, 2025
Merged

Build fix for Kafka #1079

merged 3 commits into from
Jan 8, 2025

Conversation

parmesant
Copy link
Contributor

Fixes #XXXX.

With the inclusion of crate rdkafka, build fails for targets which are not linux x86
This PR fixes that and only enables Kafka integration for linux x86 type systems

Description

This PR has:

  • been tested to ensure log ingestion and log query works.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added documentation for new or modified features or behaviors.

@coveralls
Copy link

coveralls commented Jan 7, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 12666103412

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 14 (0.0%) changed or added relevant lines in 2 files are covered.
  • 143 unchanged lines in 5 files lost coverage.
  • Overall coverage increased (+0.4%) to 12.352%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/query/stream_schema_provider.rs 0 3 0.0%
src/cli.rs 0 11 0.0%
Files with Coverage Reduction New Missed Lines %
src/main.rs 1 0.0%
src/utils/json/mod.rs 8 89.69%
src/storage/mod.rs 19 0.0%
src/metadata.rs 40 1.42%
src/storage/object_storage.rs 75 0.0%
Totals Coverage Status
Change from base Build 12652476891: 0.4%
Covered Lines: 2407
Relevant Lines: 19486
💛 - Coveralls

de-sh
de-sh reviewed Jan 7, 2025
View reviewed changes
src/query/stream_schema_provider.rs Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this related to the PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix for a compilation error on Windows

nikhilsinhaparseable
nikhilsinhaparseable previously approved these changes Jan 8, 2025
View reviewed changes
Copy link
Contributor

@nikhilsinhaparseable nikhilsinhaparseable left a comment

Choose a reason for hiding this comment

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

looks good

de-sh
de-sh reviewed Jan 8, 2025
View reviewed changes
@parmesant @de-sh
Update Cargo.toml
4bb829c 
Co-authored-by: Devdutt Shenoi <[email protected]>
Signed-off-by: parmesant <[email protected]>
@nitisht nitisht merged commit ddef8ee into parseablehq:main Jan 8, 2025
5 of 6 checks passed
parmesant added a commit to parmesant/parseable that referenced this pull request Jan 13, 2025
@parmesant @nikhilsinhaparseable @de-sh
fix: build fix for Kafka (parseablehq#1079)
5b0c004 
Signed-off-by: parmesant <[email protected]>
Co-authored-by: Nikhil Sinha <[email protected]>
Co-authored-by: Devdutt Shenoi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikhilsinhaparseable nikhilsinhaparseable nikhilsinhaparseable left review comments

+1 more reviewer

@de-sh de-sh de-sh approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
for next release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@parmesant @coveralls @de-sh @nikhilsinhaparseable @nitisht