Skip to content

Conversation

amitsrivastava
Copy link
Collaborator

@amitsrivastava amitsrivastava commented May 1, 2024

The problem is that connector based query execution is not able to
reuse session to fetch results. The frontend is sending the correct
session_id but our session fetching logic got broken when the computes
was implemented. we are now looking for the session_type from
compute['name'] for computes, connector['name'] for connector and
then snippets['type'] for old config file based hive/impala sessions.

A related change is to make use of session for get_log and check_status
calls if the frontend is sending it.

Rest is some ruff and other refactoring.

Harshg999
Harshg999 previously approved these changes May 1, 2024
Copy link
Collaborator

@Harshg999 Harshg999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, possible to add a unit test?

@amitsrivastava amitsrivastava force-pushed the dev/amit/fix_session_types_for_connectors branch from b3ceb92 to 3eff591 Compare May 2, 2024 03:28
get_interpreter, patch_snippet_for_connector

from future import standard_library
standard_library.install_aliases()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove these lines?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agl29 I have not removed it and nothing seem to have crashed this far. We have these install_aliases() at several places in our code base. We need to see if we really need them anymore.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we don't need them, may be it was required for python2 based Hue.

@amitsrivastava amitsrivastava force-pushed the dev/amit/fix_session_types_for_connectors branch from f8b4602 to f292cb3 Compare May 22, 2024 19:19
agl29
agl29 previously approved these changes May 28, 2024
@amitsrivastava amitsrivastava force-pushed the dev/amit/fix_session_types_for_connectors branch 2 times, most recently from 1df199e to 79428fb Compare December 18, 2024 19:29
The problem is that connector based query execution is not able to
reuse session to fetch results. The frontend is sending the correct
session_id but our session fetching logic got broken when the computes
was implemented. we are now looking for the session_type from
compute['name'] for computes, connector['name'] for connector and
then snippets['type'] for old config file based hive/impala sessions.

A related change is to make use of session for get_log and check_status
calls if the frontend is sending it.
@amitsrivastava amitsrivastava force-pushed the dev/amit/fix_session_types_for_connectors branch from 79428fb to dd89a0b Compare December 18, 2024 19:45
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

@amitsrivastava amitsrivastava dismissed stale reviews from agl29 and Harshg999 via dd89a0b September 22, 2025 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants