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

Azure Service Bus message receiver does not pass context to callback method #43361

Closed
2 tasks done
perry2of5 opened this issue Oct 24, 2024 · 3 comments · Fixed by #43370
Closed
2 tasks done

Azure Service Bus message receiver does not pass context to callback method #43361

perry2of5 opened this issue Oct 24, 2024 · 3 comments · Fixed by #43370
Labels
area:providers kind:bug This is a clearly a bug kind:feature Feature Requests needs-triage label for new issues that we didn't triage yet provider:microsoft-azure Azure-related issues

Comments

@perry2of5
Copy link
Contributor

Apache Airflow Provider(s)

microsoft-azure

Versions of Apache Airflow Providers

10.5.1

Apache Airflow version

2.10.2

Operating System

linux

Deployment

Docker-Compose

Deployment details

Getting-started docker deployment with PostgreSQL DB

What happened

The callback to receive an Azure Service Bus message does not include the context and so data from the message cannot be passed back through XComs.

What you think should happen instead

The callback to receive an Azure Service Bus message should include the context and so data from the message can be passed back through XComs.

How to reproduce

Method signature indicates only the message is passed into the callback. This would be more useful if the context was passed in so the callback could push some data into XComs such as the location of file from the message.

Anything else

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@perry2of5 perry2of5 added area:providers kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet labels Oct 24, 2024
Copy link

boring-cyborg bot commented Oct 24, 2024

Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.

@dosubot dosubot bot added kind:feature Feature Requests provider:microsoft-azure Azure-related issues labels Oct 24, 2024
@perry2of5
Copy link
Contributor Author

Looks like AWS S3 sensor solves this without breaking backwards compatibility by inspecting a function to determine if a context can be passed in:
https://github.com/apache/airflow/blob/main/providers/src/airflow/providers/amazon/aws/sensors/s3.py#L169

Google Cloud DataprocCreateClusterOperator does the opposite and removes an argument for backwards compatibility:
https://github.com/apache/airflow/blob/main/providers/src/airflow/providers/google/cloud/operators/dataproc.py#L680

The Snowflake provider injects the session into the operator kwargs if and only if it was passed into the function:
https://github.com/apache/airflow/blob/main/providers/src/airflow/providers/snowflake/utils/snowpark.py#L40

I think I'll fix this by allowing the callback to include the context if it wants to and then the operator can inspect the arguments and only pass the context if the callback can handle it.

@perry2of5
Copy link
Contributor Author

perry2of5 commented Oct 24, 2024

I couldn't figure out to have a type allow two different signatures so I just changed to a new signature which accepts the context. I highly doubt this will inconvenience anyone since the callback hasn't been release in the wild for more than a month and I'm, to my knowledge, the only person who tried to use it. Still, this will require a major release of the azure provider unless we want to revisit this decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers kind:bug This is a clearly a bug kind:feature Feature Requests needs-triage label for new issues that we didn't triage yet provider:microsoft-azure Azure-related issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant