Skip to content

Feature/remove session context - #14

Merged
andygrove merged 13 commits into
apache:mainfrom
timsaucer:feature/remove-session-context
Oct 19, 2024
Merged

Feature/remove session context#14
andygrove merged 13 commits into
apache:mainfrom
timsaucer:feature/remove-session-context

Conversation

@timsaucer

@timsaucertimsaucer commented Oct 3, 2024

Copy link
Copy Markdown
Member

This PR removes the datafusion-python SessionContext from being owned by Context in datafusion-ray. This allows the user to use all of the features already existing in datafusion-python.

I have tested it against the example tips.py

@timsaucer
timsaucerforce-pushed the feature/remove-session-context branch from 45364be to 89eb0a1CompareOctober 17, 2024 11:07
@timsaucer
timsaucer marked this pull request as ready for review October 18, 2024 22:05
) -> Iterable[pa.RecordBatch]:
start_time = time.time()
plan = datafusion_ray.deserialize_execution_plan(plan_bytes)
# plan = datafusion_ray.deserialize_execution_plan(plan_bytes)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: can we remove this line now?

Comment threadsrc/context.rs
Comment on lines +92 to +96
// println!("Planning {}", sql);
// let df = wait_for_future(py, self.ctx.sql(sql))?;
// let py_df = self.run_sql(sql, py)?;
// let py_plan = py_df.call_method0(py, "execution_plan")?;
// let py_plan = py_plan.bind(py);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can we remove this commented out code?

@andygroveandygrove left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Amazing. Thank you @timsaucer

@timsaucer

Copy link
Copy Markdown
MemberAuthor

Sorry I missed those commented lines. I’m out for the weekend so I’ll push the edits Monday.

@andygrove

Copy link
Copy Markdown
Member

Sorry I missed those commented lines. I’m out for the weekend so I’ll push the edits Monday.

I think is fine to fix those in a follow up PR

@andygrove
andygrove merged commit b792eb0 into apache:mainOct 19, 2024
Sign up for freeto 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.

2 participants

@timsaucer@andygrove