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

chore: disable API, GRPC, and GRPC web by default #2761

Merged
merged 2 commits into from
Oct 25, 2023

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Oct 24, 2023

Closes #2758

Testing

./scripts/single-node.sh
cat ~/.celestia-app/config/app.toml

Verified that

[api]

# Enable defines if the API server should be enabled.
enable = false
[grpc]

# Enable defines if the gRPC server should be enabled.
enable = false
[grpc-web]

# GRPCWebEnable defines if the gRPC-web should be enabled.
# NOTE: gRPC must also be enabled, otherwise, this configuration is a no-op.
enable = false

@rootulp rootulp self-assigned this Oct 24, 2023
@rootulp rootulp added the backport:v1.x PR will be backported automatically to the v1.x branch upon merging label Oct 24, 2023
@rootulp rootulp marked this pull request as ready for review October 24, 2023 14:25
Copy link
Member

@rach-id rach-id left a comment

Choose a reason for hiding this comment

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

PR looks good to me, but not sure about the rationale for this decision. Why disable them by default?

@rootulp
Copy link
Collaborator Author

rootulp commented Oct 24, 2023

Great question. I think it was motivated by this Slack thread.

Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

thanks

@evan-forbes evan-forbes merged commit b786b08 into celestiaorg:main Oct 25, 2023
28 of 29 checks passed
mergify bot pushed a commit that referenced this pull request Oct 25, 2023
Closes #2758

## Testing
```
./scripts/single-node.sh
cat ~/.celestia-app/config/app.toml
```

Verified that

```toml
[api]

# Enable defines if the API server should be enabled.
enable = false
```

```toml
[grpc]

# Enable defines if the gRPC server should be enabled.
enable = false
```

```toml
[grpc-web]

# GRPCWebEnable defines if the gRPC-web should be enabled.
# NOTE: gRPC must also be enabled, otherwise, this configuration is a no-op.
enable = false
```

(cherry picked from commit b786b08)
@rootulp rootulp deleted the rp/disable-api-by-default branch October 25, 2023 19:18
evan-forbes pushed a commit that referenced this pull request Oct 25, 2023
Closes #2758

## Testing
```
./scripts/single-node.sh
cat ~/.celestia-app/config/app.toml
```

Verified that

```toml
[api]

# Enable defines if the API server should be enabled.
enable = false
```

```toml
[grpc]

# Enable defines if the gRPC server should be enabled.
enable = false
```

```toml
[grpc-web]

# GRPCWebEnable defines if the gRPC-web should be enabled.
# NOTE: gRPC must also be enabled, otherwise, this configuration is a no-op.
enable = false
```

(cherry picked from commit b786b08)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:v1.x PR will be backported automatically to the v1.x branch upon merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

disable api, grpc and grpc-web by default in app.toml
3 participants