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

feat: lazy static runtime in python #2424

Merged
merged 8 commits into from
Apr 16, 2024

Conversation

ion-elgreco
Copy link
Collaborator

Description

As suggested by @wjones127 to create a lazy static runtime, supersedes this PR: #1950

@github-actions github-actions bot added the binding/python Issues for the Python package label Apr 16, 2024
@rtyler
Copy link
Member

rtyler commented Apr 16, 2024

Did you know! Since Rust 1.70 lazy_static is no longer needed The lazy_static rEADME even includes an example.

Since the crate is already there I don't mind its use, but thought I would mention it 😄 since the rt function could just be updated to use OnceCell

rtyler
rtyler previously approved these changes Apr 16, 2024
@ion-elgreco
Copy link
Collaborator Author

@rtyler thanks for pointing that out! :) didn't know that existed, let me make the change

wjones127
wjones127 previously approved these changes Apr 16, 2024
@ion-elgreco ion-elgreco dismissed stale reviews from wjones127 and rtyler via eacb74b April 16, 2024 15:29
@ion-elgreco
Copy link
Collaborator Author

@rtyler @wjones127 I changed it to use OnceLock and refactored the lifetime in other areas where the runtime is used

@ion-elgreco ion-elgreco enabled auto-merge (squash) April 16, 2024 15:35
@@ -25,7 +24,7 @@ pub(crate) struct FsConfig {
#[derive(Debug, Clone)]
pub struct DeltaFileSystemHandler {
pub(crate) inner: Arc<DynObjectStore>,
pub(crate) rt: Arc<Runtime>,
pub(crate) rt: Arc<&'static Runtime>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since it's a static, I don't think it even needs to hold this as part of the struct. Can just reference the global static in the methods.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or you mean to not pass it through the struct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@wjones127 I removed it from the structs

Copy link
Collaborator

@wjones127 wjones127 left a comment

Choose a reason for hiding this comment

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

Looks great!

@ion-elgreco ion-elgreco merged commit 9736522 into delta-io:main Apr 16, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binding/python Issues for the Python package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants