refactor(core): use streamer instead of stream #143
Closed
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
* add fuel-core-nats * add --nats-url CLI option * move crate * fmt * typo * update * update targets * bump versions * Cargo.lock * bail if stream height does not match chain height * appease typos check * appease typos check
* ci: improve/fix release action Now, on each push on master, a new stable release is made following semver using commit logs with Knope * ci: better commitlint config * ci: adjust rust toolchain install * ci: add wasm target
* build(repo): allow starting & stopping nats server locally * build(repo): start nats with jetstream always --------- Co-authored-by: Pedro Nauck <pedronauck@gmail.com>
* feat: bootstrap stream * feat(data-stream): refactor into Publisher struct, add payload size checks * feat(data-stream): bump deps
ci: adjust commitlint to enable dependabot
* build(repo): allow starting & stopping nats server locally * build(repo): start nats with jetstream always * fix(fuel-core-nats): remove trailing '.' in 'assets' subject * build(fuel-core-nats): automate running fuel-core-nats locally * refactor(fuel-core-nats): re-introduce command as dev/run-node * refactor(fuel-core-nats): improve fuel core nats dev script This change allows creating a .env file matching the templated environment variables from .env.sample to start the fuel-core-nats service locally.
Bumps [curve25519-dalek](https://github.com/dalek-cryptography/curve25519-dalek) from 4.1.2 to 4.1.3. - [Release notes](https://github.com/dalek-cryptography/curve25519-dalek/releases) - [Commits](dalek-cryptography/curve25519-dalek@curve25519-4.1.2...curve25519-4.1.3) --- updated-dependencies: - dependency-name: curve25519-dalek dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Pedro Nauck <pedronauck@gmail.com>
build(fuel-core-nats): introduce Docker Image for fuel-core-nats
feat: include nats server config in docker-compose
Co-authored-by: Pedro Nauck <pedronauck@gmail.com>
* feat(release): publish docker image * feat(release): update token secret name
* feat(release): add helm chart * feat(release): add helm publish workflow
…ies (#31) * ci(repo): add job for publishing docker images This commit ensures we cross-build the docker images for linux's arm and amd platforms and publish them from our Github release workflow. * fix: typo in docker publish action * build: change rust version to 1.79.0
fix: use `GITHUB_TOKEN` instead of `REPO_TOKEN`
* ci(repo): fix and improvements in release actions * ci: fix docker username on ci action * ci: add permission field on docker action * ci: update actions versions * ci: action run * ci: fix docker username for publish * ci: fix docker image publish action
* feat: introduce FuelCore trait This trait describes the interface required for `fuel-core-nats` to extend `fuel-core`. Equally, it will be useful for mocking fuel-core related components in our integration tests. * feat: add test for simple nats connections * feat: introduce NatsConnection Houses NATS connection streams, messages, and other connection details. This would eventually expose custom functions for interacting with the NATS server on a need-to -modularize basis. * feat: further scope nats' connection testing * fix: use nats configuration * feat: test nkey authorization when connecting to nats --------- Co-authored-by: Pedro Nauck <pedronauck@gmail.com>
Bumps [zerovec](https://github.com/unicode-org/icu4x) from 0.10.2 to 0.10.4. - [Release notes](https://github.com/unicode-org/icu4x/releases) - [Changelog](https://github.com/unicode-org/icu4x/blob/main/CHANGELOG.md) - [Commits](https://github.com/unicode-org/icu4x/commits/ind/zerovec@0.10.4) --- updated-dependencies: - dependency-name: zerovec dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* feat: add helmchart statefulset * feat: delete helm deployment * feat: bump chart version
feat: update working directory in dockerfile
* feat: add secretref to statefulset * feat: bump chart version * feat: set correct base image for helm chart * feat: update db_path in dockerfile * feat: create /mnt/db directory * feat: add nats user to dockerfile * feat: add user to dockerfile * feat: change owner of /mnt/db folder * feat: add init-container to statefulset * feat: revert creating db folder user in dockerfile
fix: misplaced end in helm chart
feat: bump chart version
* fix: init containers volume mounts * feat: bump chart version
* feat: define init containers from helm release * feat: bump chart version
Bumps [zerovec-derive](https://github.com/unicode-org/icu4x) from 0.10.2 to 0.10.3. - [Release notes](https://github.com/unicode-org/icu4x/releases) - [Changelog](https://github.com/unicode-org/icu4x/blob/main/CHANGELOG.md) - [Commits](https://github.com/unicode-org/icu4x/commits/ind/zerovec-derive@0.10.3) --- updated-dependencies: - dependency-name: zerovec-derive dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Pedro Nauck <pedronauck@gmail.com>
* fix(publisher): temporary solution for nats storage size * fix(publisher): repo warnings * fix(publisher): add default nats compression for streams
…ly for simplicity (#121) * refactor(core): encapsulate Subject when publishing * refactor(core): combine StreamIdentifier + StreamSubjects into new `Streamable` trait * refactor(core): model streamable traits for `fuel-core` types directly for simplicity
* refactor(core): use kv store instead of streams * fix(core): typo warnings
* feat(parser): initial commit for data-parser * feat(data-parser): cleanup * feat(repo): added prost parser * feat(repo): added criterion * feat(data-parser): extended benchmarks * feat(data-parser): added blocks and txs types * feat(data-parser): fixed Cargo.toml and lock * feat(data-parser): fixed criterion tests * feat(repo): integrated data parser in publisher * test(repo): add test project for publishing * test(repo): improve benchmarks * test(repo): improve benchmarks algorithms * fix: pre-commit warnings * test: add benchmarks * fix: clippy warnigs * docs: add readme on how to run benchs * docs: update README.md * feat(repo): integrated fuel-data-parser further in scope added benches * feat(repo): exposed selected from fuel-data-parser * feat(core): adjustments for new imports * feat(repo): improved parser unit tests and annotations * feat(core): added tx status in publisher * feat(data-parser): fixed readme * feat(repo): integrated data-parser into benchmarks * refactor(benches): some improvments to the bench runners * fix(benches): renamed Cargo.toml description * fix(data-parser): clean up of Cargo.toml * refactor(data-parser): cleanup unused deps * feat(data-parser): added publish to Cargo.toml file * feat(repo): dependencies cleanup * feat(data-parser): added entropy to data * fix(publisher): added admin profile to publisher * refactor(repo): bumped crates versions, added script * fix(publisher): use conn stores instead of arbitrary stores
* test: fix store tests filename * feat(repo): implement stream sdk * fix(repo): clippy warnings * refactor: small adjustments * refactor(core): remove unescessary trait bound * test: fix connection on tests
* feat: refactor core and publisher to integrate coherently * refactor: centralize data parsing in Streamable * refactor: restrict bincode crate to data-parser crate * refactor: fix previous tests * refactor: include minimal publishing tests * refactor: remove old comments * refactor: resolve conflicts
Jurshsmith
reviewed
Aug 16, 2024
There was a problem hiding this comment.
Looks good.
On the renaming of Stream to Streamer, I would suggest renaming the public Stream API exposed from fuel-streams as PublicStream, and then, re-export it with the alias Stream.
This might be better because:
- Explicitly naming it
PublicStreammakes us cautious of the APIs we declare there - We will read the
Streaminternally more and this will make maintenance a lot easier if the commonStreamfrom thecoreis what depicts our Streams. - The internal
StreamandPublicStreamare both streams that might implement the standard futuresStream
Please let me know what you think 🙏🏾
|
Makes sense @Jurshsmith, I'll do this when I get my computer - or if you wanna do the change in the meantime, it can be also. If you change, just warn then so I can change the PR for ready for review. |
* ci: fix release and pre-release process * ci: adjust release action * fix: ci checking
This change ensures the ambiguity between `fuel-streams-core` stream and `fuel-streams` stream is contained in the `fuel-streams` crate. In there, we will have to explicitly manage the ambiguity by accessing the internal stream with the `fuel_streams_core` namespace. Also, to prevent calling `stream.stream()` in the tests, this commit renames the `stream` field to `internal_stream` to further curb the ambiguity.
|
As discussed, I ended up keeping the Stream name in |
We can contain the `streamer` naming in `fuel_streams` for brevity
Jurshsmith
force-pushed
the
pn/refactor/stream-core
branch
from
e92a56c to
2f9780d
Compare
pedronauck
requested review from
luizstacio,
lostman and
0xterminator
as code owners
auto-merge was automatically disabled
August 16, 2024 22:04
Pull request was closed
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.
Description
I'm just rolling back what I did before this PR being merged and bringing this as a discussion to put it back in the code.
There are only two things on this PR:
Adjust the
stream.rsfile according to other modules in the core crate:It's the only one that has a different shape from other modules.
Rename the exposed
Streamcrate toStreamerI renamed it before because it can easily create confusion with another stream struct we have across the project (futures
Streamand SDKStream), and to make it work and get rid of this confusion, some extra workarounds, as @AJ did need to be implemented to don't have conflicts. So, to keep things standard and avoid extra work, makes sense to rename it toStreamer`