Skip to content

Simplify admin server feature-flagging in #1642#1682

Merged
hawkw merged 9 commits intoeliza/log-streaming-demofrom
ver/eliza/log-streaming-demo/admin-server
May 18, 2022
Merged

Simplify admin server feature-flagging in #1642#1682
hawkw merged 9 commits intoeliza/log-streaming-demofrom
ver/eliza/log-streaming-demo/admin-server

Conversation

@olix0r
Copy link
Member

@olix0r olix0r commented May 18, 2022

  • Rather than push feature flagging down into the serve logic, we can
    avoid even matching the logs.json endpoint when the feature isn't
    disabled. This avoids the need for conditional feature gatingc in the
    handler.
  • The log module is split into two submodules: level and stream. The
    stream module is feature-gated. This allows to extract function-local
    defintions into the module, which simplifies scoping and makes things
    easier to read (imo).
  • merge main

olix0r and others added 8 commits May 17, 2022 16:02
prost generates empty files for google protobuf types. This change adds
these generated files so that they are not regenerated on `make test`.

Signed-off-by: Oliver Gould <[email protected]>
Bumps [rustls](https://github.com/rustls/rustls) from 0.20.5 to 0.20.6.
- [Release notes](https://github.com/rustls/rustls/releases)
- [Changelog](https://github.com/rustls/rustls/blob/main/RELEASE_NOTES.md)
- [Commits](rustls/rustls@v/0.20.5...v/0.20.6)

---
updated-dependencies:
- dependency-name: rustls
  dependency-type: indirect
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
socket2 v0.4.5 has been yanked (rust-lang/socket2#308).

This reverts commit 9004d9c.

Signed-off-by: Oliver Gould <[email protected]>
#1678)

Bumps [EmbarkStudios/cargo-deny-action](https://github.com/EmbarkStudios/cargo-deny-action) from 1.2.17 to 1.3.0.
- [Release notes](https://github.com/EmbarkStudios/cargo-deny-action/releases)
- [Commits](EmbarkStudios/cargo-deny-action@3481b77...b655a95)

---
updated-dependencies:
- dependency-name: EmbarkStudios/cargo-deny-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Rather than push feature flagging down into the `serve` logic, we can
  avoid even matching the logs.json endpoint when the feature isn't
  disabled. This avoids the need for conditional feature gatingc in the
  handler.
* The log module is split into two submodules: level and stream. The
  stream module is feature-gated. This allows to extract function-local
  defintions into the module, which simplifies scoping and makes things
  easier to read (imo).

Signed-off-by: Oliver Gould <[email protected]>
Signed-off-by: Oliver Gould <[email protected]>
@olix0r olix0r requested a review from hawkw May 18, 2022 19:09
@olix0r olix0r requested a review from a team as a code owner May 18, 2022 19:09

// If log streaming support is enabled, start a new log stream
pub async fn serve<B>(
handle: trace::Handle,
Copy link
Member Author

Choose a reason for hiding this comment

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

Changed the type here to avoid taking a generic parameter. Cloning the handle should be cheap (and this isn't really allocation-sensitive)

Signed-off-by: Oliver Gould <[email protected]>
Copy link
Contributor

@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.

yeah, this feels a bit neater --- thanks!

@hawkw hawkw merged commit 9c764c1 into eliza/log-streaming-demo May 18, 2022
@hawkw hawkw deleted the ver/eliza/log-streaming-demo/admin-server branch May 18, 2022 20:31
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.

2 participants