docs: documented network config #35

akorchyn · 2024-12-20T14:49:16Z

race-of-sloths · 2024-12-20T14:50:04Z

@akorchyn Thank you for your contribution! Your pull request is now a part of the Race of Sloths!

Current status: waiting for merge

Reviewer	Score
@dj8yfo	5

Your contribution is much appreciated with a final score of 5!
You have received 50 Sloth points for this contribution

@dj8yfo received 25 Sloth Points for reviewing and scoring this pull request.

What is the Race of Sloths

Race of Sloths is a friendly competition where you can participate in challenges and compete with other open-source contributors within your normal workflow

For contributors:

Tag @race-of-sloths inside your pull requests
Wait for the maintainer to review and score your pull request
Check out your position in the Leaderboard
Keep weekly and monthly streaks to reach higher positions
Boast your contributions with a dynamic picture of your Profile

For maintainers:

Score pull requests that participate in the Race of Sloths and receive a reward
Engage contributors with fair scoring and fast responses so they keep their streaks
Promote the Race to the point where the Race starts promoting you
Grow the community of your contributors

Feel free to check our website for additional details!

Bot commands

For contributors
- Include a PR: @race-of-sloths include to enter the Race with your PR
For maintainers:
- Invite contributor @race-of-sloths invite to invite the contributor to participate in a race or include it, if it's already a runner.
- Assign points: @race-of-sloths score [1/2/3/5/8/13] to award points based on your assessment.
- Reject this PR: @race-of-sloths exclude to send this PR back to the drawing board.
- Exclude repo: @race-of-sloths pause to stop bot activity in this repo until @race-of-sloths unpause command is called

dj8yfo

@race-of-sloths score 5

dj8yfo · 2024-12-27T19:38:42Z

src/config.rs

    pub api_key: Option<crate::types::ApiKey>,
    /// Number of consecutive failures to move on to the next endpoint.
    pub retries: u8,
+    /// Whether to use exponential backoff between retries
+    ///
+    /// The formula is `d = initial_sleep * factor^retry`


it's left unspecified what backoff behaviour will look like if exponential_backoff is false.
it can be defined more accurately with a doc comment or
by using an enum like the following instead of exponential_backoff: bool:

pub retry_algorithm: RetryAlgorithm, .... pub enum RetryAlgorithm { ExponentialBackoff { initial_sleep: std::time::Duration, factor: u8, }, SomeOtherWay { ... }, }

dj8yfo · 2024-12-27T19:41:09Z

src/config.rs

    pub exponential_backoff: bool,
+    /// Multiplier for exponential backoff calculation
    pub factor: u8,


the backoff multiplier is somewhat simplejack instead of e.g. an f32 (where a factor can be 1.2f32),
but this is not necessarily a bad thing

Don't love floats, to be honest

dj8yfo · 2024-12-27T20:05:54Z

src/config.rs

    pub linkdrop_account_id: Option<near_primitives::types::AccountId>,
-    // https://docs.near.org/social/contract
+    /// Account ID of the [NEAR Social contract](https://docs.near.org/social/contract)


link for NEAR Social contract appears to be broken

dj8yfo · 2024-12-27T20:47:25Z

src/config.rs

    pub fn testnet() -> Self {
        Self {
            network_name: "testnet".to_string(),
            rpc_endpoints: vec![RPCEndpoint::testnet()],
            linkdrop_account_id: Some("testnet".parse().unwrap()),
            near_social_db_contract_account_id: Some("v1.social08.testnet".parse().unwrap()),
            faucet_url: Some("https://helper.nearprotocol.com/account".parse().unwrap()),
-            meta_transaction_relayer_url: Some("http://localhost:3030/relay".parse().unwrap()),
+            meta_transaction_relayer_url: None,


somewhat unrelated, but bon can be used here instead of ..NetworkConfig::mainnet() syntax,

// $ cargo add bon // bon = { version= "3.3.1", features = ["experimental-overwritable"]} use bon::Builder; use network_builder::{SetName, SetRpcStuff}; #[derive(Builder)] struct Network { name: String, #[builder(overwritable)] rpc_stuff: String, } impl Network { pub fn testnet() -> NetworkBuilder<SetRpcStuff<SetName>> { Self::builder() .name("testnet".into()) .rpc_stuff("my_fast_near_rpc".to_string()) } } fn main() { { let network = Network::testnet().build(); assert_eq!(network.name, "testnet"); assert_eq!(network.rpc_stuff, "my_fast_near_rpc"); } { let network = Network::testnet() .rpc_stuff("very_slow_another_rpc".to_owned()) .build(); assert_eq!(network.name, "testnet"); assert_eq!(network.rpc_stuff, "very_slow_another_rpc"); } }

referencing elastio/bon#149 , as it links to experimental feature needed.

without experimental-overwritable it kind of not possible to achieve the same with #[builder(default)] on the same Network type as there're different defaults for different networks

but this crate being a lib, it won't be too great to use a dependency with experimental feature, which can break on minor releases,

though pinning it with ~3.3.1 (only patch releases updates allowed) or =3.3.1 (fully pinned) would allow to avoid problems for downstream consumers of the library

To be honest, I don't really want ro pull some extra dep for that

Ideally is to remove as much deps as possible, and keep it as slim as possible

If you were to use bon builder for this, you could alternatively do it this way without the overwritable feature:

// A `#[derive(bon::Builder)]` here isn't required, but it's not prohibited! // If you want to have both `Network::builder()` for regular code and // `Network::testnet()` for test code, then both can coexist just fine struct Network { name: String, rpc_stuff: String, } // Define a builder using the method syntax. // There can be as many builders with as many different defaults as you need // for different use cases. #[bon::bon] impl Network { #[builder] pub fn testnet( #[builder(default = "testnet".into())] name: String, #[builder(default = "my_fast_near_rpc".into())] rpc_stuff: String, ) -> Self { Self { name, rpc_stuff } } } // Both of these work as expected Network::testnet().build(); Network::testnet().rpc_stuff("very_slow_another_rpc".to_owned()).build(); // If you were to add `#[derive(bon::Builder)]`, you'd also have this one. // Here you'd probably make both `name` and `rpc_stuff` required: // Network::builder().name("foo".into()).rpc_stuff("bar".into()).build();

cc @dj8yfo

@akorchyn well, this dependency has a useful function, it will be pulled in exactly for that and nothing more, it's really convenient to configure options with this stuff, i've tried. Besides Veetaha just showed how to do this with stable features of bon. Anyway, this was just a suggestion how stuff could be improved, not how it must be improved imo.
After having near-*-0.28 in, bon wouldn't be too much of additional weight, cannot compile near-workspaces tests --workspace with default number of builders, only small number -j2 works, othewise it hangs and is later killed with oom.

dj8yfo · 2024-12-27T21:09:12Z

src/config.rs

            staking_pools_factory_account_id: None,
        }
    }
 }

 #[derive(Debug)]
+/// Represents the possible outcomes of a retryable operation.
 pub enum RetryResponse<R, E> {


this type doesn't really bubble up to be eventually visible in documentation, does it

For now, I just not really sure if it's needed to be public. But I kept documentation just in case I will decide to make it public

dj8yfo · 2024-12-27T21:10:32Z

src/config.rs

+///
+/// # Arguments
+/// * `network` - The network configuration to use for the retryable operation.
+/// * `task` - The task to retry.
 pub async fn retry<R, E, T, F>(network: NetworkConfig, mut task: F) -> Result<R, RetryError<E>>


same here, it doesn't really show up in documentation

chore: network config docs

6c64863

akorchyn requested review from dj8yfo and PolyProgrammist as code owners December 20, 2024 14:49

akorchyn changed the title ~~chore!: network config docs~~ chore: network config docs Dec 20, 2024

restored fastnear for now

4fc1782

akorchyn changed the title ~~chore: network config docs~~ docs: network config docs Dec 22, 2024

akorchyn changed the title ~~docs: network config docs~~ docs: documented network config Dec 22, 2024

dj8yfo approved these changes Dec 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: documented network config #35

docs: documented network config #35

akorchyn commented Dec 20, 2024 •

edited

Loading

race-of-sloths commented Dec 20, 2024 •

edited

Loading

dj8yfo left a comment

dj8yfo Dec 27, 2024

dj8yfo Dec 27, 2024

akorchyn Dec 27, 2024

dj8yfo Dec 27, 2024

dj8yfo Dec 27, 2024

dj8yfo Dec 27, 2024

akorchyn Dec 27, 2024 •

edited

Loading

Veetaha Dec 27, 2024 •

edited

Loading

dj8yfo Dec 28, 2024 •

edited

Loading

dj8yfo Dec 27, 2024

akorchyn Dec 27, 2024

dj8yfo Dec 27, 2024

docs: documented network config #35

Are you sure you want to change the base?

docs: documented network config #35

Conversation

akorchyn commented Dec 20, 2024 • edited Loading

race-of-sloths commented Dec 20, 2024 • edited Loading

dj8yfo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akorchyn Dec 27, 2024 • edited Loading

Choose a reason for hiding this comment

Veetaha Dec 27, 2024 • edited Loading

Choose a reason for hiding this comment

dj8yfo Dec 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akorchyn commented Dec 20, 2024 •

edited

Loading

race-of-sloths commented Dec 20, 2024 •

edited

Loading

akorchyn Dec 27, 2024 •

edited

Loading

Veetaha Dec 27, 2024 •

edited

Loading

dj8yfo Dec 28, 2024 •

edited

Loading