-
Notifications
You must be signed in to change notification settings - Fork 3
Add bootstrap and bundle ValidateLedger contract #261
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
Add bootstrap and bundle ValidateLedger contract #261
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @jnmt, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a significant enhancement to the ScalarDL client by implementing a new "bootstrap" mechanism. This feature automates the essential initial setup steps, specifically the registration of the client's identity (certificate or secret key) and the ValidateLedger
system contract. This automation occurs by default when a ClientService
instance is initialized, greatly reducing manual configuration effort. For command-line users, a new bootstrap
command has been added to the ScalarDL CLI, providing a unified and simplified way to perform these registrations. The ValidateLedger
contract itself, which enables comprehensive ledger validation by scanning assets based on ID and age, is now bundled with the client, ensuring a more self-contained and user-friendly experience.
Highlights
- Automated Client Setup: Identity (certificate or secret key) and the
ValidateLedger
contract are now automatically registered when aClientService
instance is created, simplifying initial client configuration. - New
bootstrap
CLI Command: A dedicatedbootstrap
command has been added to the ScalarDL CLI, allowing users to register identity and theValidateLedger
contract with a single, convenient command. - Bundled
ValidateLedger
Contract: TheValidateLedger
system contract, imported fromscalardl-java-client-sdk
, is now bundled directly within the client, ensuring its availability for ledger validation.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands on the current page.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a bootstrap mechanism to automatically register identity and the ValidateLedger
system contract. The feature is well-integrated, with a new bootstrap
CLI command and default activation when creating a ClientService
. The changes are extensive, affecting the client service, its factory, and numerous CLI tool classes, and are accompanied by thorough tests. My review identifies one high-severity issue in the error handling logic within the new bootstrap()
method, which could mask unexpected errors by being too broad in the exceptions it suppresses. I've provided a code suggestion to correct this.
Class<?> clazz = ValidateLedger.class; | ||
try { | ||
registerContract( | ||
config.getAuditorLinearizableValidationContractId(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this version (the first version that bundles the ValidateLedger
contract), we register the validation contract with the same fixed contract ID that we have used before, for backward compatibility.
} | ||
} | ||
|
||
clientService.bootstrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By calling bootstrap()
of the primitive client service, we can simplify bootstrap()
for both table and hash store to register cert/secret and the ValidateLedger
contract.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!
Description
This PR adds the bootstrap interface and command that registers identity and
ValidateLedger
contract (imported from the scalardl-java-client-sdk repository and bundled).Different from
TableStoreClientService
andHashStoreClientService
,ClientService
does not automatically bootstrap by default for backward compatibility. By using thebootstrap
interface or command instead ofregister-certificate
andregister-secret
, users don't have to care about theValidateLedger
contract. Maybe we can discuss whether we should deprecateregister-certificate
andregister-secret
in 4.x.Related issues and/or PRs
Changes made
ClientService
, which registers identity and the ValidateLedger contractChecklist
Additional notes (optional)
N/A
Release notes
Added a bootstrap feature that registers identity and
ValidateLedger
contract.