Skip to content

Conversation

@byteZorvin
Copy link
Member

@byteZorvin byteZorvin commented Nov 12, 2025

Pull Request type

Please add the labels corresponding to the type of changes your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build-related changes
  • Documentation content changes
  • Testing
  • Other (please describe):

What is the current behavior?

  • Orchestrator does not have support for custom version constants

Resolves: #NA

What is the new behavior?

  • Orchestrator has custom version constant support

Does this introduce a breaking change?

Other information

@Mohiiit Mohiiit added the orchestrator This change is relevant to orchestrator label Nov 12, 2025
Copy link
Member

@Mohiiit Mohiiit left a comment

Choose a reason for hiding this comment

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

instead of taking the path and then converting the yaml/json file to the versionconstants, we can do something like this:

fn parse_constants(path: &str) -> Result<VersionedConstants, String> {
    std::fs::read_to_string(path)
        .map_err(|e| e.to_string())
        .and_then(|content| {
            serde_json::from_str::<VersionedConstants>(&content)
                .map_err(|e| e.to_string())
        })
}

#[derive(Parser, Debug)]
pub struct Args {
    #[arg(
        env = "MADARA_ORCHESTRATOR_VERSIONED_CONSTANTS_PATH",
        long,
        value_parser = parse_constants
    )]
    pub versioned_constants: Option<VersionedConstants>,
}

this way we won't have to add versioned_constants in the ConfigParam and it would be part of the snos_config only and can be used directly then

@byteZorvin byteZorvin marked this pull request as ready for review November 20, 2025 14:25
@byteZorvin byteZorvin enabled auto-merge November 21, 2025 12:48
@byteZorvin byteZorvin added this pull request to the merge queue Nov 21, 2025
Merged via the queue into main with commit d165ac9 Nov 21, 2025
29 checks passed
@github-project-automation github-project-automation bot moved this to Done in Madara Nov 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

orchestrator This change is relevant to orchestrator

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants