Skip to content

Converge azure.functions.ServiceBusMessage with azure.servicebus.ServiceBusMessage #1172

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

Closed
metamoof opened this issue Feb 3, 2023 · 9 comments

Comments

@metamoof
Copy link

metamoof commented Feb 3, 2023

This is copied over from Azure/azure-sdk-for-python#26827 as it was suggested I should post it in here.

I deal with a lot of code in azure functions that requires me to process messages in a ServiceBus queue, and then pass that message on untouched. However, despite the fact that an Azure Functions ServiceBus Queue Trigger passes in an azure.functions.ServiceBusMessage, azure.servicebus.ServiceBusSender does not accept that kind of message, preferring to throw the rather cryptic error: Exception: TypeError: Only AmqpAnnotatedMessage, ServiceBusMessage instances or Mappings representing messages are supported. Received instead: ServiceBusMessage.

And as such, my code is peppered with this boilerplate:

# msg is my azure.functions.ServiceBusMessage passed in at the start
with azure.servicebus.ServiceBusClient.from_connection_string(
    os.environ["SERVICEBUS"]
) as sb:
    sender = sb.get_queue_sender("next-step")
    message = azure.servicebus.ServiceBusMessage(
        body=msg.get_body(),
        application_properties=msg.user_properties,
        message_id=msg.message_id,
        content_type=msg.content_type,
    )
    sender.send_messages(message)

Also, knowing when to use application_properties or user_properties drives me a little crazy, and means I can't just do something like message = azure.servicebus.ServiceBusMessage(**msg.__dict__) or somesuch.

Describe the solution you'd like
Ideally, some sort of convergence, in which azure.functions.ServiceBusMessage is a subclass of azure.servicebus.ServiceBusMessage, but implementing the old API as backwards-compatible aliases, and eventually deprecating the APIs that are not in azure.servicebus.ServiceBusMessage. I can see no reason why they need to be separate classes.

Describe alternatives you've considered
Somewhat messier alternatives:

  • Allow azure.servicebus.ServiceBusSender.send_messages() to accept azure.functions.ServiceBusMessage instances
  • Make an azure.servicebus.ServiceBusMessage.from_azure_functions_message() constructor that creates one from the other
@bhagyshricompany bhagyshricompany self-assigned this Feb 6, 2023
@YunchuWang
Copy link
Member

YunchuWang commented Feb 6, 2023

@metamoof thanks for reporting the issue, can you try build the azure.servicebus.ServiceBusMessage from azure.functions.ServiceBusMessage which contains all attributes of azure.servicebus.ServiceBusMessage ( ref of azure.functions.ServiceBusMessage: https://github.com/Azure/azure-functions-python-library/blob/bd11d48b19dd226876ae96f7ca26a91ffa8beff6/azure/functions/servicebus.py#L8 ). Servicebus and functions package does not have dependency on each other and it is not a good idea for us to do so.

@metamoof
Copy link
Author

metamoof commented Feb 6, 2023

I can understand why you do not wish to depend on the servicebus package, but it would be nice if you both had at the very least the same Abstract Base Classes.

That being said, and being pragmatic, here's a quick survey of what is in both classes (not including the deprecated or not implemented properties):

property Functions ServiceBus
application_properties  
session_id
message_id
content_type
correlation_id
to
reply_to
reply_to_session_id
subject  
scheduled_enqueue_time_utc
time_to_live
partition_key
__str__  
__repr__  
_build_message  
_set_message_annotations  
_to_outgoing_message  
raw_amqp_message  
body  
body_type  
get_body  
dead_letter_source  
delivery_count  
enqueued_time_utc  
expires_at_utc  
label  
lock_token  
sequence_number  
user_properties  
metadata  

So, at first glance, to get API compatible, it seems all I need to do is alias application_properties and user_properties, and body to get_body. I also suspect that subject and label are the same thing - I'll write a quick script to test that out. I am not quite sure what to do with body_type - maybe raise a NotImplementedError?

The more difficult part is to make it so that the servicebus SDK will accept a Functions ServiceBusMessage - and that sems to hinge somewhat on the _to_outgoing_message method of the servicebus class, which creates an AMQP message. @YunchuWang I assume you're not particularly interested in depending on the AMQP library either?

So the solution that comes to me is to take advantage of the fact that send_messages will accept a Mapping type. I could either get ServiceBusMessage to implement collections.abc.Mapping, which is my preferred option, or as a secondary option add an as_mapping() method to ServiceBusMessage that I could then use the output of to feed into the send_messages method of the ServiceBusClient. As a bonus, both of these approaches should feed into a ServiceBusMessageBatch just fine. @YunchuWang which approach would you prefer?

Finally, whilst I am going to create some self-contained unit tests for this, I'd appreciate guidance on how to include integration tests that will run against the servicebus SDK. I'd like to ensure that the libraries remain compatible over time.

@vrdmr vrdmr transferred this issue from Azure/Azure-Functions Feb 6, 2023
@bhagyshricompany
Copy link

@metamoof pls share the all repro steps.
Thanks

@metamoof
Copy link
Author

metamoof commented Feb 7, 2023

In a function with a servicebus trigger that has a servicebus configured in the SERVICEBUS environment variable, and there is a next-step queue in that servicebus.

import azure.functions as func
import azure.servicebus
import os 

def main(msg: func.ServiceBusMessage):
    with azure.servicebus.ServiceBusClient.from_connection_string(
        os.environ["SERVICEBUS"]
    ) as sb:
        sender = sb.get_queue_sender("next-step")
        sender.send_messages(msg) # Exception: TypeError: Only AmqpAnnotatedMessage, ServiceBusMessage instances or Mappings representing messages are supported. Received instead: ServiceBusMessage

@bhagyshricompany
Copy link

thanks will check update asap

@bhagyshricompany
Copy link

@gavin-aguiar pls comment and validate

@gavin-aguiar
Copy link
Contributor

Currently this is not supported in python functions. We are working on a feature where the sdk client will be passed as a binding which should help in this case. But the feature is still in progress and will be coming out within a few months.

@alecglen
Copy link

@gavin-aguiar @bhagyshricompany, any updates for either metamoof's proposal or the SDK client binding Gavin mentioned?

As far as I can tell there is still no solution provided - not sure why it was marked completed.

@alecglen
Copy link

@gavin-aguiar I think this is the feature you were referring to?

It seems it still does not support the Service Bus SDK. Even if it did, that would not solve this issue as the provided SDK client still wouldn't accept an azure.functions.ServiceBusMessage type object.

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

No branches or pull requests

5 participants