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

update rust packages to latest #298

Merged
merged 3 commits into from
Oct 30, 2022

Conversation

suhailmalik07
Copy link
Contributor

@suhailmalik07 suhailmalik07 commented Oct 28, 2022

Description

This PR #291
Updates Rust packages to latest.

Testing

  • Integration_tests(Passed)
  • Request per sec is similar to old version (Tested on build)

PS: I'm new to rust, if there is anything which can be improved or not correct. Let me know.

@netlify
Copy link

netlify bot commented Oct 28, 2022

Deploy Preview for robyn canceled.

Name Link
🔨 Latest commit 8044d64
🔍 Latest deploy log https://app.netlify.com/sites/robyn/deploys/635e4668920b8700087216e8

let startup_handler = self.startup_handler.clone();
let shutdown_handler = self.shutdown_handler.clone();

let task_locals = Arc::new(pyo3_asyncio::TaskLocals::new(&event_loop).copy_context(py)?);
let task_locals_cleanup = task_locals.clone();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

as PyO3 update there APIs to use task_locals instead of event_loop.

Copy link
Member

Choose a reason for hiding this comment

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

@suhailmalik07 , this looks great thanks. Can you just point me to the task_locals documentation too?

Copy link
Contributor Author

@suhailmalik07 suhailmalik07 Oct 28, 2022

Choose a reason for hiding this comment

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

Sure, In documentation. It's mentioned under solution.
Link: https://docs.rs/pyo3-asyncio/0.17.0/pyo3_asyncio/#the-solution

Here in migration, It's also mentioned (Last point).

image

Link -> https://crates.io/crates/pyo3-asyncio

@suhailmalik07 suhailmalik07 marked this pull request as ready for review October 28, 2022 10:45
@suhailmalik07
Copy link
Contributor Author

This PR addresses #291 also.

@suhailmalik07
Copy link
Contributor Author

Hey @sansyrox, We can merge this one, if everything looks good to you.
Want to include this to Hacktoberfest 😅.

Cargo.toml Outdated
@@ -1,6 +1,6 @@
[package]
name = "robyn"
version = "0.18.1"
version = "0.18.1000001"
Copy link
Member

Choose a reason for hiding this comment

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

Can you replace this with 0.18.1 before we can merge the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

pyproject.toml Outdated
@@ -1,5 +1,5 @@
name = "robyn"
version = "0.18.1"
version = "0.18.1000001"
Copy link
Member

Choose a reason for hiding this comment

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

Same with this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@sansyrox sansyrox left a comment

Choose a reason for hiding this comment

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

LGTM! Just two minor suggestions.

Copy link
Member

@sansyrox sansyrox left a comment

Choose a reason for hiding this comment

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

LGTM! Great work! ❇️ 🚀

@sansyrox sansyrox merged commit 6f65643 into sparckles:main Oct 30, 2022
@suhailmalik07 suhailmalik07 deleted the update-packages branch October 31, 2022 07:10
Shending-Help pushed a commit to Shending-Help/robyn that referenced this pull request Nov 3, 2022
* update rust packages to latest

* fix clippy issues

* fix package version
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.

None yet

2 participants