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

Improved configuration API with TOML file #519

Merged
merged 67 commits into from
Aug 25, 2023
Merged

Improved configuration API with TOML file #519

merged 67 commits into from
Aug 25, 2023

Conversation

adzialocha
Copy link
Member

@adzialocha adzialocha commented Aug 22, 2023

Closes #511, #513, #516, #510

📋 Checklist

  • Add tests that cover your changes
  • Add this PR to the Unreleased section in CHANGELOG.md
  • Link this PR to any issues it closes
  • New files contain a SPDX license header

@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

Patch coverage: 79.74% and project coverage change: +0.17% 🎉

Comparison is base (5dda823) 92.16% compared to head (d7c8149) 92.34%.

❗ Current head d7c8149 differs from pull request most recent head 626277c. Consider uploading reports for the commit 626277c to get more accurate results

Additional details and impacted files
@@                Coverage Diff                @@
##           announcements     #519      +/-   ##
=================================================
+ Coverage          92.16%   92.34%   +0.17%     
=================================================
  Files                101      102       +1     
  Lines              16079    16063      -16     
=================================================
+ Hits               14820    14834      +14     
+ Misses              1259     1229      -30     
Files Changed Coverage Δ
aquadoggo/src/lib.rs 85.71% <ø> (ø)
aquadoggo/src/network/service.rs 29.83% <27.27%> (+0.62%) ⬆️
aquadoggo/src/network/behaviour.rs 45.74% <50.00%> (+6.79%) ⬆️
aquadoggo/src/config.rs 80.00% <71.42%> (+38.13%) ⬆️
aquadoggo/src/http/service.rs 96.29% <88.88%> (-0.97%) ⬇️
aquadoggo/src/network/config.rs 100.00% <100.00%> (ø)
aquadoggo/src/network/utils.rs 100.00% <100.00%> (ø)
aquadoggo/src/node.rs 88.70% <100.00%> (-0.69%) ⬇️
aquadoggo/src/replication/ingest.rs 89.47% <100.00%> (ø)
aquadoggo/src/replication/service.rs 76.25% <100.00%> (ø)
... and 2 more

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jadehopepunk
Copy link

I'm not going to police the language in anyone's code, but some teams I'm in have moved away from the whitelist/blacklist terminology to allowlist/denylist

@adzialocha
Copy link
Member Author

I'm not going to police the language in anyone's code, but some teams I'm in have moved away from the whitelist/blacklist terminology to allowlist/denylist

All really helpful feedback, thank you! Agree with that weird terminology.

Copy link
Member

@sandreae sandreae left a comment

Choose a reason for hiding this comment

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

This is looking really great to me, I had a suggestion around being able to configure your node to not support any schema ids.

Am i correct understanding that the default for a node without any custom configurations is that it is "ephemeral" (no db or identity state is persisted)?

aquadoggo_cli/config.toml Outdated Show resolved Hide resolved
aquadoggo_cli/config.toml Outdated Show resolved Hide resolved
@sandreae
Copy link
Member

Thanks for your comments @jadehopepunk all really useful!

@adzialocha adzialocha marked this pull request as ready for review August 25, 2023 09:55
@adzialocha adzialocha requested a review from sandreae August 25, 2023 09:55
Base automatically changed from announcements to main August 25, 2023 10:15
* main:
  Announce supported schema ids in network before replication (#515)
@adzialocha adzialocha merged commit 51d37b6 into main Aug 25, 2023
@adzialocha adzialocha deleted the config-galore branch August 25, 2023 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants