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

Validator should be able to be configured to use GCP storage for checkpoints #4069

Closed
Tracked by #3321
tkporter opened this issue Jun 27, 2024 · 5 comments
Closed
Tracked by #3321
Assignees
Labels
agent good first issue Good for newcomers

Comments

@tkporter
Copy link
Collaborator

tkporter commented Jun 27, 2024

Problem

  • GCP checkpoint storage is implemented and the relayer is seemingly capable of reading checkpoints
  • Validator settings seem to not support this case though

Solution

  • Update the match that validators use to support using GCP storage
    fn parse_checkpoint_syncer(syncer: ValueParser) -> ConfigResult<CheckpointSyncerConf> {
    let mut err = ConfigParsingError::default();
    let syncer_type = syncer.chain(&mut err).get_key("type").parse_string().end();
    match syncer_type {
    Some("localStorage") => {
    let path = syncer
    .chain(&mut err)
    .get_key("path")
    .parse_from_str("Expected checkpoint syncer file path")
    .end();
    cfg_unwrap_all!(&syncer.cwp, err: [path]);
    err.into_result(CheckpointSyncerConf::LocalStorage { path })
    }
    Some("s3") => {
    let bucket = syncer
    .chain(&mut err)
    .get_key("bucket")
    .parse_string()
    .end()
    .map(str::to_owned);
    let region = syncer
    .chain(&mut err)
    .get_key("region")
    .parse_from_str("Expected aws region")
    .end();
    let folder = syncer
    .chain(&mut err)
    .get_opt_key("folder")
    .parse_string()
    .end()
    .map(str::to_owned);
    cfg_unwrap_all!(&syncer.cwp, err: [bucket, region]);
    err.into_result(CheckpointSyncerConf::S3 {
    bucket,
    region,
    folder,
    })
    }
    Some(_) => {
    Err(eyre!("Unknown checkpoint syncer type")).into_config_result(|| &syncer.cwp + "type")
    }
    None => Err(err),
    }
  • Probably get rid of the Some(_) branch so this doesn't happen again
  • Probably worth a run to confirm it actually works, and maybe another pass on the code
@polymawutor
Copy link

@tkporter can i pick this up? this would be my first.

@daniel-savu
Copy link
Contributor

we should also update out infra announce-validators.ts script to parse gcp paths and announce with that:

if (location.startsWith('s3://')) {

but this is probably for another ticket

@daniel-savu
Copy link
Contributor

@tkporter can i pick this up? this would be my first.

go for it @mawutory

@polymawutor
Copy link

thanks @daniel-savu here's what i believe is expected of me:

replace Some(_) with Some("gcp") having a similar structure as Some("s3")?

@daniel-savu
Copy link
Contributor

closed in #3156 and other follow ups

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants