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

chore: log parsing pipelines happy path integration test #3424

Merged
merged 9 commits into from
Sep 2, 2023
Merged

chore: log parsing pipelines happy path integration test #3424

merged 9 commits into from
Sep 2, 2023

Conversation

raj-k-singh
Copy link
Collaborator

@raj-k-singh raj-k-singh commented Aug 24, 2023 •
edited
Loading

Contributes towards #1779

@request-info
Copy link

request-info bot commented Aug 24, 2023

We would appreciate it if you could provide us with more info about this issue/pr!

@github-actions
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@github-actions github-actions bot added the chore label Aug 24, 2023
@raj-k-singh raj-k-singh force-pushed the chore/log-pipelines-integration-test-happy-path branch from f609cc9 to 964bc57 Compare August 25, 2023 04:50
@raj-k-singh raj-k-singh marked this pull request as ready for review August 25, 2023 04:50
@srikanthccv
Copy link
Member

I haven't had a chance to work/review/collaborate with these parts of code for quite a long. I will take some time to provide any meaningful review. But feel free to merge if you Nitya approves it.

@raj-k-singh
Copy link
Collaborator Author

@nityanandagohain please review this PR.

nityanandagohain
nityanandagohain reviewed Aug 26, 2023
View reviewed changes
Copy link
Member

@nityanandagohain nityanandagohain left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, please look at the comments.

Also make sure to to include these tests in makefile so that we can run make tests

pkg/query-service/app/opamp/opamp_server.go Show resolved Hide resolved
pkg/query-service/tests/integration/logparsingpipeline_test.go Outdated Show resolved Hide resolved
pkg/query-service/tests/integration/logparsingpipeline_test.go Outdated Show resolved Hide resolved
@raj-k-singh raj-k-singh force-pushed the chore/log-pipelines-integration-test-happy-path branch from c942482 to f493a5f Compare August 28, 2023 12:00
nityanandagohain
nityanandagohain approved these changes Sep 1, 2023
View reviewed changes
Copy link
Member

@nityanandagohain nityanandagohain left a comment •
edited
Loading

Choose a reason for hiding this comment

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

It's good to merge,

I will have a look again if something needs to be added/changed and can address in new PR.

srikanthccv
srikanthccv approved these changes Sep 2, 2023
View reviewed changes
Copy link
Member

@srikanthccv srikanthccv left a comment

Choose a reason for hiding this comment

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

One approval should be enough. Approving to get this merged.

@raj-k-singh raj-k-singh merged commit 03acc33 into develop Sep 2, 2023
6 of 8 checks passed
@raj-k-singh raj-k-singh deleted the chore/log-pipelines-integration-test-happy-path branch September 2, 2023 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nityanandagohain nityanandagohain nityanandagohain approved these changes

@srikanthccv srikanthccv srikanthccv approved these changes

Assignees
No one assigned
Labels
chore request-more-info
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@raj-k-singh @srikanthccv @nityanandagohain