oxidecomputer / oxidecomputer/omicron

External API `FromStr`s could have `api::external::Error` as their error type

Open
#8,238 2 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
572
Forks
97
Avg merge
2d 12h
Merged PRs (30d)
96

Description

https://github.com/oxidecomputer/omicron/blob/80e992229b52cecb517a59a0f686f8916549343f/common/src/api/external/mod.rs#L1968-L1994

The FromStr impls in our external API mostly return Strings with some anyhow::Errors. We could converge on omicron_common::api::external::Error::InvalidValue instead, to give a little more context where more complex strings are handled.

#8194 converts the types used in firewall rules to this errorkind, but we could maybe go further here. Most of the types here have straightforward parsing, but types like Digest might benefit since they apply a similar tag:value pattern.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with common/src/api/external/mod.rs lines 1968-1994 and compare the firewall-rule conversions from #8194. Read common/src/api/external/error.rs around Error::InvalidValue, then identify which external API FromStr implementations should converge on it. Done means the selected parsers consistently use the error kind while preserving useful context for complex values such as Digest.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
api
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.