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

Add an option to specify topic name for nodes_report_cloud_segments #16925

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

rpdevmp
Copy link
Contributor

@rpdevmp rpdevmp commented Mar 7, 2024

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.3.x
  • v23.2.x

Improvements

Improvements to internal high thougput tests to be able to specify and count topic-specific segments

Release Notes

  • none
    This is internal testing HTT and release notes not required.

NOTE: This has NOT been tested locally yet. I would need to pull this code to my AWS EC2 instance and test it. Submitting PR for initial discussion and comments

@CLAassistant
Copy link

CLAassistant commented Mar 7, 2024

CLA assistant check
All committers have signed the CLA.

@vbotbuildovich
Copy link
Collaborator

@rpdevmp
Copy link
Contributor Author

rpdevmp commented Mar 7, 2024

Buildkite shows as passed:
https://buildkite.com/redpanda/redpanda/builds/45767#018e168d-2180-40c4-837b-5420135f4ad1

But when I clicked on details, I see that 1 test actually failed:
https://ci-artifacts.dev.vectorized.cloud/redpanda/45767/018e168d-2180-40c4-837b-5420135f4ad1/vbuild/ducktape/results/2024-03-07--001/report.html

Also, I already signed the license and got redirected to GH. If I click details and try to sign again, it says I already signed it. But for some reason this check is still pending even if I click "recheck"

@rpdevmp
Copy link
Contributor Author

rpdevmp commented Mar 7, 2024

OK, looks like I had different name and email set in local Git.. Updated it globally and would need to fix this tomorrow so license check would pass

savex
savex previously approved these changes Mar 7, 2024
Copy link
Contributor

@savex savex left a comment

Choose a reason for hiding this comment

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

lgtm on green build and fixed linter errors

travisdowns
travisdowns previously approved these changes Mar 8, 2024
Copy link
Member

@dotnwat dotnwat left a comment

Choose a reason for hiding this comment

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

I don't see any reference in upstream dev or in this PR to the aws_ip_ranges.json file. Am I missing something? If not, can we remove it from this PR and add it when it's used? Also, can we avoid committing it at all since its >60K lines long?

@rpdevmp
Copy link
Contributor Author

rpdevmp commented Mar 8, 2024

I don't see any reference in upstream dev or in this PR to the aws_ip_ranges.json file. Am I missing something? If not, can we remove it from this PR and add it when it's used? Also, can we avoid committing it at all since its >60K lines long?

Yes, I will remove it. It would be used in another task and also we might host it somewhere else and avoid committing due to file size. Thanks for pointing this out.

self.logger.info(f"Getting the first 100 segments into the cloud")
wait_until(lambda: nodes_report_cloud_segments(self.redpanda, 100),
self.logger.info(
f"Getting the first 100 segments into the cloud for specific topic"
Copy link
Member

Choose a reason for hiding this comment

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

this f-string doesn't contain any interpolated values. did you intend to add some extra context to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, it was like this before, I just modified it a bit. For now should be good

@savex savex self-requested a review March 14, 2024 19:32
Copy link
Contributor

@savex savex left a comment

Choose a reason for hiding this comment

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

lgtm on green build

@travisdowns
Copy link
Member

Forced: due to ghost bk-redpanda issue.

@travisdowns travisdowns merged commit 057524c into dev Mar 14, 2024
16 checks passed
@travisdowns travisdowns deleted the rpdevmp/iptables-989 branch March 14, 2024 19:41
@rpdevmp rpdevmp mentioned this pull request Mar 14, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants