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

cluster: move archival into cluster library #19955

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

dotnwat
Copy link
Member

@dotnwat dotnwat commented Jun 22, 2024

This PR relocates archival into the cluster library to reflect the reality that it was built as part of v::cluster. While build systems like CMake are fine with this chaos, others are more strict.

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
  • v24.1.x
  • v23.3.x
  • v23.2.x

Release Notes

  • none

@vbotbuildovich
Copy link
Collaborator

new failures in https://buildkite.com/redpanda/redpanda/builds/50542#01903e51-ee46-4d6b-b222-a24791f0b076:

"rptest.tests.crl_test.CertificateRevocationTest.test_pp_api"

@vbotbuildovich
Copy link
Collaborator

@dotnwat dotnwat requested a review from oleiman June 22, 2024 14:47
This move relocates archival into the cluster library to reflect the
reality that it was built as part of v::cluster. While build systems
like CMake are fine with this chaos, others are more strict.

Signed-off-by: Noah Watkins <[email protected]>
@dotnwat
Copy link
Member Author

dotnwat commented Jun 22, 2024

Force pushed to fix merge conflict.

@Lazin
Copy link
Contributor

Lazin commented Jun 22, 2024 via email

@Lazin
Copy link
Contributor

Lazin commented Jun 22, 2024 via email

@dotnwat
Copy link
Member Author

dotnwat commented Jun 22, 2024

I have a lot of code for the archival subsystem in the feature branch/PRs.

I'm happy to hold off on this PR and wait for your stuff to merge.

I'm also don't think archival should be a part of the cluster.

Unless that's going to happen very soon, let's move it. It's been sitting there a long time, and it's going to get in the way of Bazel. It's a trivial matter to move it back when someone dedicates time to pulling things apart.

Copy link
Member

@oleiman oleiman left a comment

Choose a reason for hiding this comment

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

seems legit, though I'm probably biased toward bazel enablement

@Lazin
Copy link
Contributor

Lazin commented Jun 23, 2024

It's a trivial matter to move it back when someone dedicates time to pulling things apart.

I'd really love to use bazel (actually anything which is not cmake) but once we move all the code into one library the code will gradually get more and more entangled.

@dotnwat
Copy link
Member Author

dotnwat commented Jun 25, 2024

I'd really love to use bazel (actually anything which is not cmake) but once we move all the code into one library the code will gradually get more and more entangled.

i don't think it is realistic to hold up bazel because we'd like archival to be separated from cluster. it's compiled into the cluster library today, but the files are physically separated, and that physical separation isn't preventing the situation from getting worse. someone needs to take the time to separate them if that is the outcome we are shooting for.

@dotnwat dotnwat marked this pull request as draft June 25, 2024 18:48
@dotnwat
Copy link
Member Author

dotnwat commented Jun 25, 2024

Converting to draft until @Lazin's PRs are merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants