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.

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

Clarifications and improvements to Kafka manual testing documentation #422

Merged
13 commits merged into from
Nov 14, 2022
Merged

Clarifications and improvements to Kafka manual testing documentation #422

13 commits merged into from
Nov 14, 2022

Conversation

dagardner-nv
Copy link
Contributor

@dagardner-nv dagardner-nv commented Oct 27, 2022 •
edited
Loading

Fixes #322

@dagardner-nv dagardner-nv added non-breaking Non-breaking change doc Improvements or additions to documentation 3 - Ready for Review labels Oct 27, 2022
@dagardner-nv dagardner-nv requested a review from a team as a code owner October 27, 2022 21:11
mdemoret-nv
mdemoret-nv approved these changes Nov 2, 2022
View reviewed changes
Copy link
Contributor

@mdemoret-nv mdemoret-nv 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. Thanks for the updates.

pdmack
pdmack requested changes Nov 8, 2022
View reviewed changes
Copy link
Contributor

@pdmack pdmack left a comment

Choose a reason for hiding this comment

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

@dagardner-nv @mdemoret-nv I wonder if the testing instructions should just stop at the basic copy, partition tests instead of the pipelines which are covered elsewhere. There's been a fair amount of divergence (e,g., HAMMAH->DFP) and I guess I'm worried that this will be a bit of a maintenance headache.

scripts/validation/kafka_testing.md Show resolved Hide resolved
scripts/validation/kafka_testing.md Show resolved Hide resolved
scripts/validation/kafka_testing.md Show resolved Hide resolved
scripts/validation/kafka_testing.md Show resolved Hide resolved
scripts/validation/kafka_testing.md Outdated Show resolved Hide resolved
scripts/validation/kafka_testing.md Outdated Show resolved Hide resolved
scripts/validation/kafka_testing.md Outdated Show resolved Hide resolved
@dagardner-nv
Copy link
Contributor Author

I wonder if the testing instructions should just stop at the basic copy, partition tests instead of the pipelines which are covered elsewhere.

I think there is some truth to this, when I wrote the document I wanted to mimic each of the four pipelines in the scripts/validation directory. However the Kakfa version of those four pipelines have been codified into tests that are now run as a part of CI.

pdmack
pdmack approved these changes Nov 14, 2022
View reviewed changes
Copy link
Contributor

@pdmack pdmack left a comment

Choose a reason for hiding this comment

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

LGTM
Very clear @dagardner-nv !

@pdmack
Copy link
Contributor

pdmack commented Nov 14, 2022

@gpucibot merge

@ghost ghost merged commit 276b72e into nv-morpheus:branch-22.11 Nov 14, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pdmack pdmack pdmack approved these changes

@mdemoret-nv mdemoret-nv mdemoret-nv approved these changes

Assignees
No one assigned
Labels
doc Improvements or additions to documentation non-breaking Non-breaking change
Projects
Morpheus Boards
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[DOC] Kafka manual testing clarifications and improvements
3 participants
@dagardner-nv @pdmack @mdemoret-nv