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

pandaproxy/sr: bazelize compatibility_protobuf test #24657

Merged
merged 2 commits into from
Jan 13, 2025

Conversation

IoannisRP
Copy link
Contributor

@IoannisRP IoannisRP commented Dec 24, 2024

Implements: CORE-7549 - compatibility_protobuf.cc

Had to change how the known types .proto files are ingested. Without the changes in the schema_registry_protos bazel target, they were visible using the full local path from workspace. i.e.

import "src/v/pandaproxy/schema_registry/protobuf/google/protobuf/any.proto";

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

Release Notes

  • none

@IoannisRP IoannisRP force-pushed the ik-bazel-tests-pandaproxy-p2 branch 5 times, most recently from 215d783 to e7a8052 Compare January 10, 2025 11:46
@IoannisRP IoannisRP force-pushed the ik-bazel-tests-pandaproxy-p2 branch from e7a8052 to e40d475 Compare January 10, 2025 11:47
@IoannisRP IoannisRP marked this pull request as ready for review January 10, 2025 11:54
@IoannisRP IoannisRP requested review from BenPope and rockwotj January 10, 2025 11:54
Copy link
Member

@BenPope BenPope left a comment

Choose a reason for hiding this comment

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

I wonder if it's worth moving the well-known types from pandaproxy/BUILD to schema_registry/protobuf, as well?

Or maybe into /src/v/schema/protobuf?

I tried to package up confluent_proto, google_type_proto, and well_known_proto into their own cc_libraries, but failed to get that to work.

src/v/pandaproxy/schema_registry/protobuf/BUILD Outdated Show resolved Hide resolved
@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Jan 10, 2025

CI test results

test results on build#60587
test_id test_kind job_url test_status passed
idempotency_tests_rpunit.idempotency_tests_rpunit unit https://buildkite.com/redpanda/redpanda/builds/60587#01945015-8290-4e6e-af01-e2d7f07604e5 FLAKY 1/2
rm_stm_tests_rpunit.rm_stm_tests_rpunit unit https://buildkite.com/redpanda/redpanda/builds/60587#01945015-8290-4e6e-af01-e2d7f07604e5 FLAKY 1/2
rm_stm_tests_rpunit.rm_stm_tests_rpunit unit https://buildkite.com/redpanda/redpanda/builds/60587#01945015-8291-4c0e-9c44-9c932242e0be FLAKY 1/2
rptest.tests.partition_reassignments_test.PartitionReassignmentsTest.test_reassignments_kafka_cli ducktape https://buildkite.com/redpanda/redpanda/builds/60587#0194505e-5eb7-4e41-8fcb-5b355e830bee FLAKY 3/6
rptest.tests.scaling_up_test.ScalingUpTest.test_scaling_up_with_recovered_topic ducktape https://buildkite.com/redpanda/redpanda/builds/60587#01945061-c49b-460a-9f83-c60713e5b442 FLAKY 5/6
rptest.transactions.stream_verifier_test.StreamVerifierTest.test_simple_produce_consume_txn_with_add_node ducktape https://buildkite.com/redpanda/redpanda/builds/60587#0194505e-5eb6-4da2-b1c1-61e84d8db16e FLAKY 5/6
test results on build#60624
test_id test_kind job_url test_status passed
rptest.tests.partition_force_reconfiguration_test.NodeWiseRecoveryTest.test_node_wise_recovery.dead_node_count=1 ducktape https://buildkite.com/redpanda/redpanda/builds/60624#01945df6-f630-4b63-9b8a-04d8d115b348 FLAKY 5/6

Don't depend on the protobuf library, then to get raw `.proto` files
you have to know about how the `proto_library` rule works, instead just
create a filegroup rule for the protos that the go test depends on,
which is also consumed by the `proto_library` rule.
@IoannisRP IoannisRP merged commit 5e40521 into redpanda-data:dev Jan 13, 2025
24 checks passed
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.

4 participants