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

Do not generate the operation registry when there are no operations #1369

Closed
wants to merge 6 commits into from

Conversation

david-perez
Copy link
Contributor

Do not generate the operation registry, nor the operation handler
traits, when there are no operations in the service closure.

Testing

Without this commit, we codegen a crate for the empty.smithy service that
cannot be compiled.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Do not generate the operation registry, nor the operation handler
traits, when there are no operations in the service closure.
@david-perez david-perez requested a review from a team as a code owner May 6, 2022 14:32
@github-actions
Copy link

github-actions bot commented May 6, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

Rust Wrk benchmark report:

Duration: 90 sec, Connections: 32, Threads: 2

Measurement Deviation Current Old
Requests/sec 0.07% 38107.65 38082.48
Total requests 0.07% 3431875 3429462
Total errors NaN% 0 0
Total successes 0.07% 3431875 3429462
Average latency ms 0.00% 0.87 0.87
Minimum latency ms 0.00% 0.02 0.02
Maximum latency ms 20.40% 20.24 16.81
Stdev latency ms -5.26% 0.54 0.57
Transfer Mb 0.07% 356.75 356.49
Connect errors NaN% 0 0
Read errors NaN% 0 0
Write errors NaN% 0 0
Status errors (not 2xx/3xx) NaN% 0 0
Timeout errors NaN% 0 0

@david-perez
Copy link
Contributor Author

There's no harm in adding the test to the client too I guess.

@david-perez david-perez requested a review from a team as a code owner June 8, 2022 17:23
@github-actions
Copy link

github-actions bot commented Jun 8, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

Rust Wrk benchmark report:

Duration: 90 sec, Connections: 32, Threads: 2

Measurement Deviation Current Old
Requests/sec 0.42% 75566.66 75248.56
Total requests 0.51% 6808491 6774032
Total errors NaN% 0 0
Total successes 0.51% 6808491 6774032
Average latency ms 24.53% 0.66 0.53
Minimum latency ms 0.00% 0.02 0.02
Maximum latency ms -4.51% 16.5 17.28
Stdev latency ms 26.51% 1.05 0.83
Transfer Mb 0.51% 707.75 704.16
Connect errors NaN% 0 0
Read errors NaN% 0 0
Write errors NaN% 0 0
Status errors (not 2xx/3xx) NaN% 0 0
Timeout errors NaN% 0 0

@github-actions
Copy link

github-actions bot commented Jun 8, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

Rust Wrk benchmark report:

Duration: 90 sec, Connections: 32, Threads: 2

Measurement Deviation Current Old
Requests/sec 2.57% 42096.49 41042.44
Total requests 2.57% 3789133 3694094
Total errors NaN% 0 0
Total successes 2.57% 3789133 3694094
Average latency ms 3.75% 0.83 0.8
Minimum latency ms 0.00% 0.02 0.02
Maximum latency ms -17.00% 16.65 20.06
Stdev latency ms 30.19% 0.69 0.53
Transfer Mb 2.57% 393.88 384
Connect errors NaN% 0 0
Read errors NaN% 0 0
Write errors NaN% 0 0
Status errors (not 2xx/3xx) NaN% 0 0
Timeout errors NaN% 0 0

@david-perez david-perez force-pushed the davidpz/empty-service branch from 0d0ea61 to 7948da7 Compare June 9, 2022 11:26
@github-actions
Copy link

github-actions bot commented Jun 9, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

Rust Wrk benchmark report:

Duration: 90 sec, Connections: 32, Threads: 2

Measurement Deviation Current Old
Requests/sec -0.20% 61642.87 61763.33
Total requests -0.25% 5550714 5564351
Total errors NaN% 0 0
Total successes -0.25% 5550714 5564351
Average latency ms -1.64% 0.6 0.61
Minimum latency ms 0.00% 0.02 0.02
Maximum latency ms 7.25% 17.76 16.56
Stdev latency ms 1.45% 0.7 0.69
Transfer Mb -0.25% 577 578.42
Connect errors NaN% 0 0
Read errors NaN% 0 0
Write errors NaN% 0 0
Status errors (not 2xx/3xx) NaN% 0 0
Timeout errors NaN% 0 0

@github-actions
Copy link

github-actions bot commented Jun 9, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

Copy link
Collaborator

@rcoh rcoh left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@rcoh rcoh 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 make this fail

@david-perez
Copy link
Contributor Author

I'll work on making it throw an exception instead of logging a warning.

@LukeMathWalker
Copy link
Contributor

I assume we can close this since the registry is gone? @david-perez

@hlbarber
Copy link
Contributor

Closed in favor of #2351

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
server Rust server SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants