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

[AppGateServer] Refactor the AppGate server to use the new shannon-sdk implementation #636

Merged
merged 12 commits into from
Jun 28, 2024

Conversation

red-0ne
Copy link
Contributor

@red-0ne red-0ne commented Jun 27, 2024

Summary

Import and use the new shannon-sdk implementation to implement the AppGateServer.

Type of change

Select one or more:

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Documentation
  • Other (specify)

Testing

Documentation changes (only if making doc changes)

  • make docusaurus_start; only needed if you make doc changes

Local Testing (only if making code changes)

  • Unit Tests: make go_develop_and_test
  • LocalNet E2E Tests: make test_e2e
  • See quickstart guide for instructions

PR Testing (only if making code changes)

  • DevNet E2E Tests: Add the devnet-test-e2e label to the PR.
    • THIS IS VERY EXPENSIVE, so only do it after all the reviews are complete.
    • Optionally run make trigger_ci if you want to re-trigger tests without any code changes
    • If tests fail, try re-running failed tests only using the GitHub UI as shown here

Sanity Checklist

  • I have tested my changes using the available tooling
  • I have commented my code
  • I have performed a self-review of my own code; both comments & source code
  • I create and reference any new tickets, if applicable
  • I have left TODOs throughout the codebase, if applicable

@red-0ne red-0ne added off-chain Off-chain business logic sdk Everything relatd to the sdk labels Jun 27, 2024
@red-0ne red-0ne self-assigned this Jun 27, 2024
@red-0ne red-0ne changed the base branch from main to refactor/ring-address-height June 27, 2024 16:26
@red-0ne red-0ne changed the base branch from refactor/ring-address-height to main June 27, 2024 17:27
pkg/appgateserver/endpoint_selector.go Outdated Show resolved Hide resolved
pkg/appgateserver/endpoint_selector.go Outdated Show resolved Hide resolved
pkg/appgateserver/endpoint_selector.go Outdated Show resolved Hide resolved
pkg/appgateserver/endpoint_selector.go Outdated Show resolved Hide resolved
pkg/appgateserver/sdkadapter/block_client.go Show resolved Hide resolved
pkg/appgateserver/sdkadapter/sdk.go Outdated Show resolved Hide resolved
pkg/appgateserver/sdkadapter/sdk.go Show resolved Hide resolved
pkg/appgateserver/sdkadapter/sdk.go Outdated Show resolved Hide resolved
pkg/crypto/rings/client.go Outdated Show resolved Hide resolved
pkg/crypto/rings/client.go Outdated Show resolved Hide resolved
Copy link
Contributor

@bryanchriswhite bryanchriswhite left a comment

Choose a reason for hiding this comment

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

Preemptively approving 🔥 🚀 - only 1 question this time. 😅

// Filter out the supplier endpoints that match the requested serviceId.
validSupplierEndpoints := make([]*sdktypes.SingleSupplierEndpoint, 0, len(supplierEndpoints))
) (supplierEndpoint shannonsdk.Endpoint, err error) {
endpoints, err := sessionEndpoints.AllEndpoints()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why use #AllEndpoints() here instead of adding some filter function(s) - maybe #PUC?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

	// AppGateServer uses the custom getRelayerUrl instead of leveraging the SDK's
	// filter to select the next endpoint to use.
	// This is because it needs to maintain the state of the last selected endpoint
	// and have a view on the original request URL to determine the next endpoint.
	// This behavior is specific to the AppGateServer and needed by clients that
	// need to instrument the endpoint selection strategy, such as the Load testing tool.

Copy link

The CI will now also run the e2e tests on devnet, which increases the time it takes to complete all CI checks.

You may need to run make trigger_ci to submit an empty commit that'll trigger the tests.

GCP workloads (requires changing the namespace to 636)
Grafana network dashboard for devnet-issue-{issue-id}

@github-actions github-actions bot added devnet push-image CI related - pushes images to ghcr.io labels Jun 28, 2024
@red-0ne red-0ne merged commit ca45b0f into main Jun 28, 2024
10 checks passed
bryanchriswhite added a commit that referenced this pull request Jun 28, 2024
…tor+feat

* pokt/main:
  [Proof Module] on-chain claim/proof message distribution validation (#620)
  [AppGateServer] Refactor the AppGate server to use the new shannon-sdk implementation  (#636)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devnet devnet-test-e2e off-chain Off-chain business logic push-image CI related - pushes images to ghcr.io sdk Everything relatd to the sdk
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants