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

[C++][FlightRPC] Make warning about do_action stream less noisy #34047

Closed
marsupialtail opened this issue Feb 5, 2023 · 5 comments · Fixed by #34182
Closed

[C++][FlightRPC] Make warning about do_action stream less noisy #34047

marsupialtail opened this issue Feb 5, 2023 · 5 comments · Fixed by #34182

Comments

@marsupialtail
Copy link
Contributor

Describe the bug, including details regarding any error messages, version, and platform.

Hi @lidavidm pyarrow 11.0 is breaking my arrow flight related code. I think you have a good sense what the problem is: /arrow/cpp/src/arrow/status.cc:137: DoAction result was not fully consumed: Cancelled: Flight cancelled call, with message: CANCELLED. Detail: Cancelled

I seem to be fully consuming the DoAction results, e.g. here is my code:

for result in self.flight_client.do_action(action):
      assert result.body.to_pybytes().decode("utf-8") == "True"

Can you give some pointers on how to use Python to get results from do_action?

Component(s)

FlightRPC

@lidavidm
Copy link
Member

lidavidm commented Feb 5, 2023 via email

@marsupialtail
Copy link
Contributor Author

Can I disable this warning?

@lidavidm
Copy link
Member

lidavidm commented Feb 5, 2023

Does list(do_action(...)) work?

I think a PR to remove the warning or change it to DEBUG level would be reasonable, it's probably a bit aggressive as-is.

@lidavidm lidavidm changed the title Flight server problems with the streaming results in 11.0 [C++][FlightRPC ]Flight server problems with the streaming results in 11.0 Feb 5, 2023
@lidavidm lidavidm changed the title [C++][FlightRPC ]Flight server problems with the streaming results in 11.0 [C++][FlightRPC] Flight server problems with the streaming results in 11.0 Feb 5, 2023
@marsupialtail
Copy link
Contributor Author

OK doing list() seems to work. But I still think this shouldn't be the default :-)

@lidavidm
Copy link
Member

lidavidm commented Feb 5, 2023

It was basically a conflict between people who want this to be a proper iterator (in which case gRPC really wants you to drain the stream) and the original behavior which was to eagerly materialize the list.

@marsupialtail do you want to leave this open? We could change it to not warn by default, at least.

@lidavidm lidavidm reopened this Feb 6, 2023
@lidavidm lidavidm changed the title [C++][FlightRPC] Flight server problems with the streaming results in 11.0 [C++][FlightRPC] Make warning about do_action stream less noisy Feb 6, 2023
lidavidm added a commit that referenced this issue Feb 17, 2023
### Rationale for this change

The warning added for when you fail to consume a result stream (and get the final error/success result) is a bit aggressive. Ignore it in most cases.

### What changes are included in this PR?

Drop the warning to DEBUG level and don't log if it's just a cancellation.

### Are these changes tested?

Existing tests cover it.

### Are there any user-facing changes?

Users should see less inactionable warnings.
* Closes: #34047

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
@lidavidm lidavidm added this to the 12.0.0 milestone Feb 17, 2023
fatemehp pushed a commit to fatemehp/arrow that referenced this issue Feb 24, 2023
…apache#34182)

### Rationale for this change

The warning added for when you fail to consume a result stream (and get the final error/success result) is a bit aggressive. Ignore it in most cases.

### What changes are included in this PR?

Drop the warning to DEBUG level and don't log if it's just a cancellation.

### Are these changes tested?

Existing tests cover it.

### Are there any user-facing changes?

Users should see less inactionable warnings.
* Closes: apache#34047

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants