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

load_shed: make constructor for Overloaded error public #661

Merged
merged 3 commits into from Jun 13, 2022

Conversation

mattklein123
Copy link
Contributor

This allows for mocking. This also matches what is done for
the timeout Elapsed error.

Signed-off-by: Matt Klein mklein@lyft.com

This allows for mocking. This also matches what is done for
the timeout Elapsed error.

Signed-off-by: Matt Klein <mklein@lyft.com>
@mattklein123
Copy link
Contributor Author

Any chance of getting this reviewed/merged? Thank you!

Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

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

Sorry for the delay on this! I was out on vacation :D

Generally, we don't want to expose these constructors but we already do this for the Elapsed error in the timeout layer so I am fine with merging this.

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

This seems fine to me.

In general, we try to avoid exposing error type constructors when they're not necessary, as it poses a forward-compatibilty issue (we may want to add additional data to the error in the future), but in this case, it seems pretty unlikely, and if it's useful for testing, we may as well make it public.

tower/src/load_shed/error.rs Show resolved Hide resolved
Matt Klein added 2 commits June 12, 2022 14:54
@hawkw hawkw merged commit 3c170aa into tower-rs:master Jun 13, 2022
@mattklein123 mattklein123 deleted the overloaded-public branch June 13, 2022 21:34
@mattklein123
Copy link
Contributor Author

Awesome thank you for the merge!

hawkw pushed a commit that referenced this pull request Jun 17, 2022
This allows for mocking. This also matches what is done for
the timeout Elapsed error.

Signed-off-by: Matt Klein <mklein@lyft.com>
hawkw pushed a commit that referenced this pull request Jun 17, 2022
This allows for mocking. This also matches what is done for
the timeout Elapsed error.

Signed-off-by: Matt Klein <mklein@lyft.com>
hawkw added a commit that referenced this pull request Jun 17, 2022
# 0.4.13 (June 17, 2022)

### Added

- **load_shed**: Public constructor for `Overloaded` error ([#661])

### Fixed

- **util**: Fix hang with `call_all` when the `Stream` of requests is
  pending ([#656])
- **ready_cache**: Ensure cancelation is observed by pending services
  ([#668], fixes [#415])
- **docs**: Fix a missing section header due to a typo ([#646])
- **docs**: Fix broken links to `Service` trait ([#659])

[#661]: #661
[#656]: #656
[#668]: #668
[#415]: #415
[#646]: #646
[#659]: #659
hawkw added a commit that referenced this pull request Jun 17, 2022
# 0.4.13 (June 17, 2022)

### Added

- **load_shed**: Public constructor for `Overloaded` error ([#661])

### Fixed

- **util**: Fix hang with `call_all` when the `Stream` of requests is
  pending ([#656])
- **ready_cache**: Ensure cancelation is observed by pending services
  ([#668], fixes [#415])
- **docs**: Fix a missing section header due to a typo ([#646])
- **docs**: Fix broken links to `Service` trait ([#659])

[#661]: #661
[#656]: #656
[#668]: #668
[#415]: #415
[#646]: #646
[#659]: #659
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

3 participants