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

Feature: add Response class #1431

Closed
Lancetnik opened this issue May 8, 2024 · 5 comments · Fixed by #1481
Closed

Feature: add Response class #1431

Lancetnik opened this issue May 8, 2024 · 5 comments · Fixed by #1481
Labels
Core Issues related to core FastStream functionality and affects to all brokers enhancement New feature or request

Comments

@Lancetnik
Copy link
Member

We should create a special class to make users able to setup outgoing message metainformation in @publihser decorator case

@broker.publisher("out")
@broker.subscriber("in")
async def handler(msg) -> Reponse:
    return Reponse(data=b"", headers={})

Also, the basic case with just a data should be supported as well

@broker.publisher("out")
@broker.subscriber("in")
async def handler(msg):
    return b""   # should works

More than we should create specific KafkaResponse/RabbitResponse/... classes to allow user to specify broker-specific information like in the following example

@kafka_broker.publisher("out")
@kafka_broker.subscriber("in")
async def handler(msg) -> Reponse:
    return KafkaResponse(data=b"", headers={}, key=b"")

P.S. RMQ ReplyConfig option should be deprecated

@AmanSal1
Copy link

AmanSal1 commented May 9, 2024

@Lancetnik Can I give this a shot? Could you just brief me a little bit about this?

@Lancetnik Lancetnik removed the good first issue Good for newcomers label May 10, 2024
@Lancetnik
Copy link
Member Author

@AmanSal1 sorry, I was a little hasty. It's not so easy to implement as I thought. It requires coordinated changes in all publishers, so I'll implement it by myself.

If you want to help, you create create a basic Response class in broker/response.py with __new__ method, that could consume raw data or Response class instance and return Response itself if it was passed or create a new one from data (you can take a look here for reference)

@AmanSal1
Copy link

@Lancetnik

class Response:
    def __new__(cls, *args, **kwargs):
        if len(args) == 1 and isinstance(args[0], cls):
            # If an instance of Response is passed, return it directly
            return args[0]
        else:
            # Otherwise, create a new instance with the provided data
            return super().__new__(cls)

    def __init__(self, data=None, headers=None, key=None):
        self.data = data
        self.headers = headers or {}
        self.key = key

Could you please check if this is what you meant? For the next part, I suppose we would need to create separate classes for each and inherit the response class for them. Then, we would have to modify the decorator class for each response class

@Lancetnik
Copy link
Member Author

Yeah, something like this

@Lancetnik Lancetnik added the Core Issues related to core FastStream functionality and affects to all brokers label May 16, 2024
Lancetnik added a commit that referenced this issue May 28, 2024
@Lancetnik
Copy link
Member Author

@AmanSal1 thank you for the interest! I make a draft release of Response classes, as u can see, but we still need to specify broker-specific options in [Broker]Response class. You can take care on it, if you want

github-merge-queue bot pushed a commit that referenced this issue May 29, 2024
* feat (#1431): add Response class

* docs: add Response References

* docs: add Confluent Response API
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Issues related to core FastStream functionality and affects to all brokers enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants