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.

Sign up for GitHub

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

Jump to bottom

Add Rust SDK async/await syntax support and minor improvements #1733

Merged
merged 3 commits into from
Aug 6, 2020
Merged

Add Rust SDK async/await syntax support and minor improvements #1733

merged 3 commits into from
Aug 6, 2020

Conversation

yoshd
Copy link
Contributor

@yoshd yoshd commented Aug 2, 2020 •
edited
Loading

What type of PR is this?

/kind feature

What this PR does / Why we need it:

Added functions that supports async/await syntax.
The existing API can be used as it is without breaking it.
This allows the following asynchronous calls:

let sdk = agones::Sdk::new_async().await?;

sdk.ready_async().await?;
sdk.set_label_async("test-label", "test-value").await?;
sdk.set_annotation_async("test-annotation", "test value").await?;
let gameserver = sdk.get_gameserver_async().await?;
// Other functions are similar...

Also, in order to resolve #1201, I fixed grpcio's openssl feature to allow it to pass through.

+[features]
+openssl = ["grpcio/openssl"]
+openssl-vendored = ["grpcio/openssl-vendored"]

I updated the Rust edition to 2018 and also updated the other dependent crate versions.
Other minor cleanups have also been done so that warnings will no longer appear at build time.

Which issue(s) this PR fixes:
Fixes #1732, #1201

Relates to #1300

@yoshd
Copy link
Contributor Author

yoshd commented Aug 2, 2020

/assign @EricFortin

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: be9f9fe3-b1d9-4f51-96e0-6d20d27b208f

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/googleforgames/agones.git pull/1733/head:pr_1733 && git checkout pr_1733
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.8.0-4fcde59

aLekSer
aLekSer reviewed Aug 3, 2020
View reviewed changes
build/includes/sdk.mk Outdated Show resolved Hide resolved
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: f1c9ed8d-4471-4066-a4f0-78132ea314dd

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@aLekSer
Copy link
Collaborator

aLekSer commented Aug 4, 2020 •
edited
Loading

As a side note make hugo-test has failed:

fatal error: concurrent map read and map write

goroutine 150 [running]:
runtime.throw(0x1e2fd5a, 0x21)
	/usr/local/go/src/runtime/panic.go:1116 +0x72 fp=0xc002bf1428 sp=0xc002bf13f8 pc=0x4f1712
runtime.mapaccess1_faststr(0x1b27300, 0xc000c7b770, 0xc00033332f, 0x18, 0xc0007d5d68)
	/usr/local/go/src/runtime/map_faststr.go:21 +0x43c fp=0xc002bf1498 sp=0xc002bf1428 pc=0x4cfc1c
github.com/gohugoio/hugo/tpl/internal/go_templates/texttemplate.(*state).walkTemplate(0xc002bf1b78, 0x1d8ce20, 0xc002ee8b70, 0x194, 0xc000987f80)
	/root/project/hugo/tpl/internal/go_templates/texttemplate/exec.go:399 +0x8e fp=0xc002bf1580 sp=0xc002bf1498 pc=0xf0403e
github.com/gohugoio/hugo/tpl/internal/go_templates/texttemplate.(*state).walk(0xc002bf1b78, 0x1d8ce20, 0xc002ee8b70, 0x194, 0x2369400, 0xc000987f80)

Which is tracked under gohugoio/hugo#7293 .

markmandel
markmandel reviewed Aug 4, 2020
View reviewed changes
Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

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

Looks good to me, just would like clarification on std-async vs tokio -- we probably don't want to tie ourselves to one of the other 😬 ?

sdks/rust/Cargo.toml
error-chain = "0.11.0"
grpcio = "0.6.0"
protobuf = "2.16.2"
futures = { version = "0.3", features = ["compat"] }
Copy link
Member

Choose a reason for hiding this comment

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

In theory, this should work with either tokio or std-async, yeah?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's right.
run-sdk-conformance-test is working with async-std.

I've tried the sample that works with tokio in the following branch and confirmed that it works.
Should I commit such an example to this PR?
https://github.com/yoshd/agones/blob/a740ed653b72ed3d1a05accbf790f076af991090/examples/rust-async-simple/src/main.rs

Copy link
Member

Choose a reason for hiding this comment

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

No I don't think so - just wanted to make sure 👍

Sounds like this is good to go 👍

markmandel
markmandel approved these changes Aug 5, 2020
View reviewed changes
Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

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

Let's do it!

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: markmandel, yoshd

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-robot
Copy link

New changes are detected. LGTM label has been removed.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 049e634d-7de9-4f29-aa2e-6c8cda4debce

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/googleforgames/agones.git pull/1733/head:pr_1733 && git checkout pr_1733
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.8.0-74c3143

@markmandel markmandel merged commit 3f44b58 into googleforgames:master Aug 6, 2020
@markmandel markmandel added this to the 1.8.0 milestone Aug 11, 2020
@markmandel markmandel added area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/feature New features for Agones labels Aug 11, 2020
ilkercelikyilmaz pushed a commit to ilkercelikyilmaz/agones that referenced this pull request Oct 23, 2020
@yoshd @markmandel @ilkercelikyilmaz
Add Rust SDK async/await syntax support and minor improvements (googl…
4e8487d 
…eforgames#1733)

* Add Rust SDK async/await syntax support and minor improvements

* Fix comment in run-sdk-conformance-test-rust

Co-authored-by: Mark Mandel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aLekSer aLekSer aLekSer left review comments

@markmandel markmandel markmandel approved these changes

@cyriltovena cyriltovena Awaiting requested review from cyriltovena

@pooneh-m pooneh-m Awaiting requested review from pooneh-m

Assignees

@markmandel markmandel

@EricFortin EricFortin

Labels
approved area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc cla: yes kind/feature New features for Agones size/XXL
Projects
None yet
Milestone
1.8.0
Development

Successfully merging this pull request may close these issues.

Support for async/await syntax in Rust SDK Rust SDK conflicts with dependencies using openssl
7 participants
@yoshd @agones-bot @aLekSer @google-oss-robot @markmandel @EricFortin @googlebot