search

dbt-labs / dbt-rpc

A server that can compile and run queries in the context of a dbt project. Additionally, it provides methods that can be used to list and terminate running processes.
https://docs.getdbt.com/reference/commands/rpc
Apache License 2.0
19 stars 7 forks source link

Add new context argument to _get_responses method #118

Closed anubhavsharma515 closed 1 year ago

anubhavsharma515 commented 1 year ago

Fixes https://github.com/dbt-labs/dbt-rpc/issues/116 (and https://github.com/dbt-labs/dbt-rpc/issues/113 I believe).

From the linked issue, a recent modification to the json-rpc parent JSONRPCResponseManager https://github.com/pavlov99/json-rpc/pull/122/files#diff-71ff9d558b2e51825f517c97faa2f119ab0aaf26aaff43e36731719017cce06eR75 class adds a new argument that is causing an error in the overridden version in the custom dbt-rpc ResponseManager class.

This PR essentially just adds in the same context argument to the _get_responses method, which is being called from the parent handle_request method.

ChenyuLInx commented 1 year ago

@anubhavsharma515 Thanks for the fix!! I don't think this would resolve #113 but def resolves #116. I think internally we got over the issue by pinning a previous version of json-rpc. But this should to be the way longer term.

colin-rogers-dbt commented 1 year ago

This makes sense, need to debug the failing unit tests

anubhavsharma515 commented 1 year ago

Hey @ChenyuLInx and @colin-rogers-dbt, many thanks for the review and also for pointing out the issue with the CI tests. I reset the setup.py to what it was before the version was pinned and hopefully this is the right way to go.

Kindly let me know if there's any further changes required on my end (and thank you so much for taking the time to review again).

ChenyuLInx commented 1 year ago

@anubhavsharma515 Thank you so much for looking into this and update!! Just one last question and I am happy to merge after it is being confirmed!