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

[cli] Remove indexer feature for sui and remove the sui-pg binary #19436

Merged

Conversation

stefan-mysten
Copy link
Contributor

@stefan-mysten stefan-mysten commented Sep 18, 2024

Description

Recent improvements on GraphQL and Postgres code removed the dynamic linking to libpq. Due to this linking (GH actions was linking to postgres@14), we had to have a separate sui-pg binary that was built with the indexer feature such that one can have access to sui start --with-indexer and --with-graphql due to the required dependencies to Postgres.

This PR removes the indexer feature, all the instances of building the binary with --features indexer in workflows, actions, etc, and updates the documentation (docs + sui-test-validator crate). This is possible because libpq is no longer a dependency that gets dynamically linked to the binary. Note that in order to use those flags, a running Postgres DB is still required just like before. If you used sui-pg binary previously, you can simply use sui binary from this version on.

Test plan

Existing tests.


Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • Indexer:
  • JSON-RPC:
  • GraphQL:
  • CLI: The indexer feature was removed from the sui crate as the dynamic linking to libpq was removed. Therefore, the sui-pg binary will not be part of releases anymore. This sui-pg binary was used for starting a network with --with-indexer and --with-graphql flags. These commands will still work as before and it is still required to have installed a Postgres database.
    If you used sui-pg binary previously, you can simply use sui binary from this version on.
  • Rust SDK:
  • REST API:

Copy link

vercel bot commented Sep 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 18, 2024 9:54pm
3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Sep 18, 2024 9:54pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Sep 18, 2024 9:54pm
sui-typescript-docs ⬜️ Ignored (Inspect) Visit Preview Sep 18, 2024 9:54pm

@github-actions github-actions bot added the Type: Documentation Improvements or additions to documentation label Sep 18, 2024
Copy link
Contributor

github-actions bot commented Sep 18, 2024

⚠️ 🦋 Changesets Warning: This PR has changes to public npm packages, but does not contain a changeset. You can create a changeset easily by running pnpm changeset in the root of the Sui repo, and following the prompts. If your change does not need a changeset (e.g. a documentation-only change), you can ignore this message. This warning will be removed when a changeset is added to this pull request.

Learn more about Changesets.

@@ -29,15 +29,19 @@ runs:
- name: cargo build
if: env.s3_file_exist == '' # if empty, we have not built and uploaded this binary to s3 yet
run: |
cargo build --bin sui --features indexer
if [[ "${{ inputs.ref }}" == 'devnet' || "${{ inputs.ref }}" == 'testnet' ]]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that this is a workaround until we get this code into devnet/testnet branches. When that happens, we need to remove this (a.k.a the workflow will fail anyway).

@stefan-mysten stefan-mysten merged commit 940149d into MystenLabs:main Sep 19, 2024
43 of 45 checks passed
@stefan-mysten stefan-mysten deleted the cli_remove_indexer_feature branch September 19, 2024 00:49
ebmifa pushed a commit that referenced this pull request Sep 19, 2024
…#19436)

## Description 

Recent improvements on GraphQL and Postgres code removed the dynamic
linking to `libpq`. Due to this linking (GH actions was linking to
postgres@14), we had to have a separate `sui-pg` binary that was built
with the `indexer` feature such that one can have access to `sui start
--with-indexer` and `--with-graphql` due to the required dependencies to
Postgres.

This PR removes the `indexer` feature, all the instances of building the
binary with `--features indexer` in workflows, actions, etc, and updates
the documentation (docs + sui-test-validator crate). This is possible
because `libpq` is no longer a dependency that gets dynamically linked
to the binary. Note that in order to use those flags, a running Postgres
DB is still required just like before.

## Test plan 

Existing tests.

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [x] CLI: the `indexer` feature was removed from the `sui` crate as the
dynamic linking to `libpq` was removed. Therefore, the `sui-pg` binary
will not be part of releases anymore. This `sui-pg` binary was used for
starting a network with `--with-indexer` and `--with-graphql` flags.
These commands will still work as before and it is still required to
have installed a Postgres database.
If you used `sui-pg` binary previously, you can simply use `sui` binary
from this version on.
- [ ] Rust SDK:
- [ ] REST API:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants