Conversation
Summary of ChangesHello @JC-wk, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the message handling capabilities by introducing a 'claim check' pattern for Service Bus and Event Grid, which intelligently offloads large payloads to blob storage. It also provides flexibility by making the Service Bus SKU configurable, allowing environments to balance cost and performance. These changes ensure more robust and scalable inter-service communication. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
/gemini review |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces significant changes to support Azure Service Bus Standard SKU, primarily by refactoring the codebase to be asynchronous and implementing the claim-check pattern for handling large messages. The introduction of a supervisor pattern for Service Bus consumers is a commendable improvement for service reliability. However, I've identified a critical configuration issue in the Terraform code related to the Service Bus queue size that will cause deployment failures with the Standard SKU. Additionally, there's a high-severity maintainability concern due to significant code duplication of the new claim-check helper logic across different application components, which should be refactored into a shared library.
| async def _offload_to_blob(content: str) -> str: | ||
| account_url = f"https://{config.SERVICE_BUS_MESSAGES_STORAGE_ACCOUNT_NAME}.blob.{config.STORAGE_ENDPOINT_SUFFIX}" | ||
| async with credentials.get_credential_async_context() as credential: | ||
| blob_service_client = BlobServiceClient(account_url, credential=credential) | ||
| async with blob_service_client: | ||
| blob_name = f"msg-{uuid.uuid4()}.json" | ||
| blob_client = blob_service_client.get_blob_client(container=config.SERVICE_BUS_MESSAGES_CONTAINER_NAME, blob=blob_name) | ||
| await blob_client.upload_blob(content) | ||
| return f"{config.SERVICE_BUS_MESSAGES_CONTAINER_NAME}/{blob_name}" | ||
|
|
||
|
|
||
| async def receive_message_payload(msg: ServiceBusMessage) -> str: | ||
| body_str = b"".join(msg.body).decode("utf-8") | ||
| try: | ||
| body_json = json.loads(body_str) | ||
| if "claim_check" in body_json: | ||
| blob_path = body_json["claim_check"] | ||
| logger.info(f"Message has claim check: {blob_path}. Downloading from blob storage.") | ||
| return await _download_from_blob(blob_path) | ||
| except json.JSONDecodeError: | ||
| pass | ||
|
|
||
| return body_str | ||
|
|
||
|
|
||
| async def _download_from_blob(blob_path: str) -> str: | ||
| account_url = f"https://{config.SERVICE_BUS_MESSAGES_STORAGE_ACCOUNT_NAME}.blob.{config.STORAGE_ENDPOINT_SUFFIX}" | ||
| container_name, blob_name = blob_path.split("/", 1) | ||
| async with credentials.get_credential_async_context() as credential: | ||
| blob_service_client = BlobServiceClient(account_url, credential=credential) | ||
| async with blob_service_client: | ||
| blob_client = blob_service_client.get_blob_client(container=container_name, blob=blob_name) | ||
| download_stream = await blob_client.download_blob() | ||
| content = await download_stream.readall() | ||
| return content.decode("utf-8") |
There was a problem hiding this comment.
There is significant code duplication for the Service Bus claim-check helper functions (_offload_to_blob, receive_message_payload, _download_from_blob) across multiple components:
api_app/service_bus/helpers.pyairlock_processor/shared_code/sb_helpers.pyresource_processor/shared/sb_helpers.py
This duplication will increase maintenance overhead and could lead to inconsistencies in the future. Please consider refactoring this shared logic into a common library that all components can import.
1 participant
Description
This PR introduces the Claim-Check pattern for Azure Service Bus to handle messages that exceed the size limits of the Standard SKU (256 KB). While the
Premium SKU supports up to 100 MB, large messages (e.g., extensive deployment logs or error traces) can still pose challenges for downstream consumers
like Cosmos DB.
This implementation allows components to offload large message payloads to an Azure Storage account, sending only a reference (claim-check) in the
Service Bus message.
Key Changes
async.
Validation Results