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

[compute] check correctly for is_compute for session_type #3510

Merged
merged 2 commits into from
Oct 24, 2023

Conversation

amitsrivastava
Copy link
Collaborator

the problem is that when multiple queries are executed, we are not able to reuse the existing sessions as the session reuse is broken.

if computes are in use then use compute[name] as the type otherwise fallback to old snippet[type] and use it everywhere.

Change-Id: I0e6cb1149ca7961fc799ccce69fe70343d241bb5

the problem is that when multiple queries are executed, we are not able
to reuse the existing sessions as the session reuse is broken.

if computes are in use then use compute[name] as the type otherwise
fallback to old snippet[type] and use it everywhere.

Change-Id: I0e6cb1149ca7961fc799ccce69fe70343d241bb5
@@ -316,11 +317,11 @@ def execute(self, notebook, snippet):
db = self._get_db(snippet, interpreter=self.interpreter)

statement = self._get_current_statement(notebook, snippet)
session = self._get_session(notebook, snippet['type'])
compute = snippet.get('compute')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Snippet will always have compute? And compute will have name?

Copy link
Contributor

Choose a reason for hiding this comment

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

It can potentially be undefined in the UI:

snippet.compute && snippet.compute.id ? snippet.compute : undefined

Not sure in which scenario but perhaps when loading back old saved editor documents, from before computes were introduced. I'm guessing is_compute takes care of checking it @amitsrivastava?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now made the checks safer.

Copy link
Contributor

@JohanAhlen JohanAhlen left a comment

Choose a reason for hiding this comment

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

LGTM

… previous call.

Change-Id: Ice8f9a48344a8b6008d0c0e632ddc20948034da5
Copy link
Contributor

@JohanAhlen JohanAhlen left a comment

Choose a reason for hiding this comment

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

Nice!

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!

@amitsrivastava amitsrivastava merged commit 0e8667e into master Oct 24, 2023
5 checks passed
@amitsrivastava amitsrivastava deleted the dev/amit/multi-queries-use-db branch October 24, 2023 17:35
athithyaaselvam pushed a commit that referenced this pull request Feb 5, 2024
the problem is that when multiple queries are executed, we are not able
to reuse the existing sessions as the session reuse is broken.

if computes are in use then use compute[name] as the type otherwise
fallback to old snippet[type] and use it everywhere.

frontend update to send the session_type from the session returned
by previous call.

Change-Id: I0e6cb1149ca7961fc799ccce69fe70343d241bb5
(cherry picked from commit 0e8667e)
wing2fly pushed a commit that referenced this pull request Mar 6, 2024
…3510)

the problem is that when multiple queries are executed, we are not able
to reuse the existing sessions as the session reuse is broken.

if computes are in use then use compute[name] as the type otherwise
fallback to old snippet[type] and use it everywhere.

frontend update to send the session_type from the session returned
by previous call.

Change-Id: I0e6cb1149ca7961fc799ccce69fe70343d241bb5
(cherry picked from commit 0e8667e)
(cherry picked from commit c454852)
(cherry picked from commit b5d5802)
athithyaaselvam pushed a commit that referenced this pull request Mar 14, 2024
…3510)

the problem is that when multiple queries are executed, we are not able
to reuse the existing sessions as the session reuse is broken.

if computes are in use then use compute[name] as the type otherwise
fallback to old snippet[type] and use it everywhere.

frontend update to send the session_type from the session returned
by previous call.

Change-Id: I0e6cb1149ca7961fc799ccce69fe70343d241bb5
(cherry picked from commit 0e8667e)
(cherry picked from commit c454852)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants