-
Notifications
You must be signed in to change notification settings - Fork 32
Mj/file store helium proto #1068
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
Open
michaeldjeffrey
wants to merge
31
commits into
main
Choose a base branch
from
mj/file-store-helium-proto
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Nothing is reading/writing to csv in file-store. And the dump command follows the pattern of dumping json to stdout.
file-store proper doesn’t really do anything with settings (cli commands, but okay), to where we need to wrap a config error in a special library type. We can return a config::ConfigError and the caller can map it how they need on their application startup.
We don’t need a special library error from a cli. The internals can return file_store::Error, and we can turn them into anyhow. This is planning for being able to break the cli out into a proto specific workspace for helium proto types. cli dump command to return anyhow::Result and use MsgDecode where we can and it doesn’t confuse things
- ReportId and IngestId to iot_verifier where they’re used solely - TimestampEncode/Decode to file_store_shared
…e-helium-proto This operation is confusing because we run into a slew of rust orphan rule problems. We’re trying to keep the type we’re writing in FileSink<T>, but cannot tie it to a trait within file-store, because we don’t have enough leverage outside of file-store to implement it for anything.
This get’s rid of the need for MsgBytes, but keeps us typed over a proto message that that can serialize itself to bytes.
We can turn into FileInfo from anything that knows how to stringify itself and a DateTime.
Breaking out those shared components helped me think through a bunch of the changes that were made, but they don’t need to be their workspace anymore. remove file-store-shared
It was nice to have all that out of the way when working on refactors, but it’s not useful enough on its own to deserve a workspace. remove file-store-helium-cli remove more cli code
The CLI is application layer, and does not need to constrain itself to a file_store::Error.
doesn’t make sense to remove this one yet imho
connormck333
approved these changes
Oct 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
file-storeRefactor: Separate Helium Proto ConcernsThis refactor extracts
helium-protospecific functionality fromfile-storeinto a new crate:file-store-helium-proto. The goal is to removehelium-protoas a dependency fromfile-store.file-storeremains protobuf-specific (usingprost), but now developers can usefile-storewith their own local protos without needing to importhelium-proto. All Helium-specific protocol mappings and types are consolidated intofile-store-helium-proto.file-storeCleanupRemove MsgBytes Trait
The
MsgBytestrait has been removed fromfile-store. Previously, this trait existed to abstract over different message types that could be written to aFileSink. The trait had to be supplementally implemented for any types that wanted to useFileSinkWriteExt. NowFileSinkWriteExtcan be used with any protobuf type that implementsprost::Message.Remove
helium-protoDependencyfile-storeno longer depends onhelium-proto. As part of this change:UnknownEnumValueerror now comes fromprostinstead ofhelium-protohelium-protospecific mappings have been moved tofile-store-helium-protoGeneral Cleanup
Several unused dependencies and code were removed:
csvdependency and related error variantconfigdependency andConfigErrorvariantJoinError,ShutdownFileInfoPollerErrorto include context:{0}in error messageclap,strum,strum_macros,blake3,beacon,humantime-serdeCLI Commands Moved
All CLI commands have been moved to
file-store-helium-protoand now returnanyhow::Result.file-store-helium-protoStructure
The new crate is organized into logical groupings that mirror the Helium network structure:
iot/- IoT-specific protocol mappings (beacon reports, witness reports, packets, valid/invalid PoC)mobile/- Mobile-specific protocol mappings (coverage, sessions, speedtests, wifi heartbeats, bans, subscriber info, transfers)subscriber/- Subscriber-specific mappings (verified mapping events, unique connections, usage counts)network_common/- Cross-network shared types (entropy reports, reward manifests)file_type.rs-FileTypeenum moved fromfile-storetraits/- Helium-specific traitsTraits Moved
These traits were specific to
helium-prototypes have been moved to avoid orphan rules:MsgVerify- for verifying signed messagesFileSinkWriteExt- extension trait for writing to file sinks with Helium-specific rollover and commit strategiesMsgTimestamp- convertingu64<->DateTime<Utc>for proto <-> domain struct mappingsThe
MsgTimestamptrait remains infile-store-helium-protoastraits::MsgTimestampsince it's used across multiple Helium proto types. Moving this trait created orphan rule issues initially, but these have been resolved by implementing the trait within the same crate that defines the types.FileType Enum
The
FileTypeenum has been moved fromfile-storetofile-store-helium-protosince it's entirely Helium-specific.CLI
The CLI (
file-store-helium-protobinary) provides commands for working with Heliumfile-storedata.The
print_jsonutility function has also been moved here since it's used by the CLI commands.Codebase Updates
All downstream crates have been updated to use
file-store-helium-protofor Helium-specific types and traits:Test Updates
In test files, you'll see imports like
use file_store_helium_proto::coverageinstead of shorter paths. This is intentional - many test files have local modules namedcoverage,mobile_session, etc. Using the fully qualifiedfile_store_helium_proto::prefix makes it immediately clear which types are from the library vs. local test helpers.Import Changes
Here's an example of some common imports that may have changed.
Before:
After: