Azure / azure-functions-python-worker

Python worker for Azure Functions.
http://aka.ms/azurefunctions
MIT License
335 stars 103 forks source link

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

Closed metamoof closed 1 year ago

metamoof commented 1 year ago

This is copied over from https://github.com/Azure/azure-sdk-for-python/issues/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:

YunchuWang commented 1 year ago

@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 commented 1 year ago

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   :white_check_mark:
session_id :white_check_mark: :white_check_mark:
message_id :white_check_mark: :white_check_mark:
content_type :white_check_mark: :white_check_mark:
correlation_id :white_check_mark: :white_check_mark:
to :white_check_mark: :white_check_mark:
reply_to :white_check_mark: :white_check_mark:
reply_to_session_id :white_check_mark: :white_check_mark:
subject   :white_check_mark:
scheduled_enqueue_time_utc :white_check_mark: :white_check_mark:
time_to_live :white_check_mark: :white_check_mark:
partition_key :white_check_mark: :white_check_mark:
__str__   :white_check_mark:
__repr__   :white_check_mark:
_build_message   :white_check_mark:
_set_message_annotations   :white_check_mark:
_to_outgoing_message   :white_check_mark:
raw_amqp_message   :white_check_mark:
body   :white_check_mark:
body_type   :white_check_mark:
get_body :white_check_mark:  
dead_letter_source :white_check_mark:  
delivery_count :white_check_mark:  
enqueued_time_utc :white_check_mark:  
expires_at_utc :white_check_mark:  
label :white_check_mark:  
lock_token :white_check_mark:  
sequence_number :white_check_mark:  
user_properties :white_check_mark:  
metadata :white_check_mark:  

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.

bhagyshricompany commented 1 year ago

@metamoof pls share the all repro steps. Thanks

metamoof commented 1 year ago

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 commented 1 year ago

thanks will check update asap

bhagyshricompany commented 1 year ago

@gavin-aguiar pls comment and validate

gavin-aguiar commented 1 year ago

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 commented 1 week ago

@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 commented 1 week ago

@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.