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

Fix Kafka offset checking test #1212

Merged

Conversation

dagardner-nv
Copy link
Contributor

@dagardner-nv dagardner-nv commented Sep 21, 2023

Description

  • Fixes intermittent failures in tests/test_kafka_source_stage_pipe.py::test_kafka_source_commit
  • The test checked the offsets in a stage, however the C++ impl for the source stage performs a commit after calling on_next. This also limited us to only testing sync commits.
  • Updated test performs the offset check after the pipeline completes when we can be assured that all commits have completed.

Fxies #1217

By Submitting this PR I confirm:

  • I am familiar with the Contributing Guidelines.
  • When the PR is ready for review, new or existing tests cover these changes.
  • When the PR is ready for review, the documentation is up to date with these changes.

@dagardner-nv dagardner-nv requested a review from a team as a code owner September 21, 2023 18:52
@dagardner-nv dagardner-nv self-assigned this Sep 21, 2023
@dagardner-nv dagardner-nv added bug Something isn't working non-breaking Non-breaking change DO NOT MERGE PR should not be merged; see PR for details labels Sep 21, 2023
@dagardner-nv dagardner-nv reopened this Sep 21, 2023
… the source's yield returns.

Adjust the offset checker to pass if there is a new partition introduced.
…prior to the source's yield returns."

This reverts commit 948f4ef.
… after the pipeline completes. The reason is that the C++ impl commits the offsets after calling on_next, this also adds the ability to check async commits as well
@dagardner-nv dagardner-nv requested a review from a team as a code owner September 22, 2023 16:31
@dagardner-nv dagardner-nv changed the title Draft: Testing Kafka errors in CI Fix Kafka offset checking test Sep 22, 2023
@dagardner-nv dagardner-nv removed the DO NOT MERGE PR should not be merged; see PR for details label Sep 22, 2023
@dagardner-nv
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 13d9d8c into nv-morpheus:branch-23.11 Sep 22, 2023
5 checks passed
dagardner-nv added a commit to dagardner-nv/Morpheus that referenced this pull request Oct 4, 2023
* Fixes intermittent failures in `tests/test_kafka_source_stage_pipe.py::test_kafka_source_commit`
* The test checked the offsets in a stage, however the C++ impl for the source stage performs a commit after calling `on_next`. This also limited us to only testing sync commits.
* Updated test performs the offset check after the pipeline completes when we can be assured that all commits have completed.

Fxies nv-morpheus#1217

- I am familiar with the [Contributing Guidelines](https://github.com/nv-morpheus/Morpheus/blob/main/docs/source/developer_guide/contributing.md).
- When the PR is ready for review, new or existing tests cover these changes.
- When the PR is ready for review, the documentation is up to date with these changes.

Authors:
  - David Gardner (https://github.com/dagardner-nv)

Approvers:
  - Michael Demoret (https://github.com/mdemoret-nv)

URL: nv-morpheus#1212
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Non-breaking change
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants