Skip to content

Google Cloud Storage CheckpointSyncer implementation#3156

Merged
daniel-savu merged 15 commits intohyperlane-xyz:mainfrom
eigerco:eiger_gcs_storage_2422
Feb 7, 2024
Merged

Google Cloud Storage CheckpointSyncer implementation#3156
daniel-savu merged 15 commits intohyperlane-xyz:mainfrom
eigerco:eiger_gcs_storage_2422

Conversation

@35359595
Copy link
Contributor

Description

GSC builder and implementer classes;
Documentation comments;
Examples;
Anonymous test on public bucket access;

Drive-by changes

cargo doc build fixes:

  • replaced square braces of non-existing items with ' ;
  • raw URL surrounded by <>;

Related issues

Fixes #2242 Part 1

Backward compatibility

Yes

Testing

Unit Tests

@changeset-bot
Copy link

changeset-bot bot commented Jan 15, 2024

⚠️ No Changeset found

Latest commit: 169cf4c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@35359595 35359595 requested a review from daniel-savu January 20, 2024 04:58
@avious00
Copy link
Contributor

hey Ivan @35359595 - @daniel-savu will review again shortly. thank you for your patience!

Copy link
Contributor

@daniel-savu daniel-savu left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM

@daniel-savu
Copy link
Contributor

@35359595 looks like cargo test is failing

@35359595
Copy link
Contributor Author

@35359595 looks like cargo test is failing

Some doc tests were missing imports after comment fixes applied. Fixed now.

@35359595
Copy link
Contributor Author

35359595 commented Feb 6, 2024

@daniel-savu please re-run the CI for this.

@codecov
Copy link

codecov bot commented Feb 7, 2024

Codecov Report

Merging #3156 (169cf4c) into main (54aeb64) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3156   +/-   ##
=======================================
  Coverage   67.65%   67.65%           
=======================================
  Files          99       99           
  Lines        1014     1014           
  Branches      106      106           
=======================================
  Hits          686      686           
  Misses        284      284           
  Partials       44       44           
Components Coverage Δ
core 50.00% <ø> (ø)
hooks 68.79% <ø> (ø)
isms 65.94% <ø> (ø)
token 58.41% <ø> (ø)
middlewares 81.46% <ø> (ø)

@daniel-savu daniel-savu merged commit 65e2ea5 into hyperlane-xyz:main Feb 7, 2024
@sapph1re
Copy link
Contributor

When I try to use it, it keeps saying "Unknown checkpoint syncer type".

I see that although rust/hyperlane-base/src/settings/checkpoint_syncer.rs seems to be designed to support GCS for storage, but rust/agents/validator/src/settings.rs doesn't have it and only expects either localStorage or s3.

Is it still WIP to support Google Cloud? Or how do you specify the Checkpoint Syncer settings to use GCS bucket for storage?

@daniel-savu
Copy link
Contributor

daniel-savu commented Aug 13, 2024

hi @sapph1re, yes it seems like this feature has a bug where the validator can't be configured. See this issue for more details, although I don't really have the bandwidth to support with the fix now
#4069

@avious00
Copy link
Contributor

avious00 commented Aug 13, 2024

hi @sapph1re saw your discord message too, we're happy to create a bounty to successfully implement this if you're interested

@sapph1re
Copy link
Contributor

hi @sapph1re saw your discord message too, we're happy to create a bounty to successfully implement this if you're interested

Ok let's do that

github-merge-queue bot pushed a commit that referenced this pull request Aug 27, 2024
### Description

Fixing the issue I described
[earlier](#3156 (comment))
where the usage of GCS as an alternative to AWS although supported by
Hyperlane Agents, in practice didn't work because the settings still
didn't accept "gcs" as a checkpoint syncer type.

### Backward compatibility

Yes

### Testing

Manual
fmorency pushed a commit to fmorency/hyperlane-agents that referenced this pull request Feb 20, 2025
### Description

Fixing the issue I described
[earlier](hyperlane-xyz/hyperlane-monorepo#3156 (comment))
where the usage of GCS as an alternative to AWS although supported by
Hyperlane Agents, in practice didn't work because the settings still
didn't accept "gcs" as a checkpoint syncer type.

### Backward compatibility

Yes

### Testing

Manual
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Alternate CheckpointSyncers for agents

4 participants