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

[Session] Add support for GetSession via the CLI + deps #163

Merged
merged 4 commits into from
Nov 8, 2023

Conversation

Olshansk
Copy link
Member

@Olshansk Olshansk commented Nov 8, 2023

Summary

Human Summary

  • Update the CLI code flow to be able to call GetSession
  • Add make targets to trigger GetSession
  • Replace [] with <> in all CLI functions to differentiate required vs optional params
  • Made a couple changes to SessionHydrator (and added tests) to account additional error cases
Screen.Recording.2023-11-01.at.4.49.33.PM.mov

AI Summary

Summary generated by Reviewpad on 08 Nov 23 20:55 UTC

This pull request includes the following changes:

  1. In the file query_application.go, the command show-application now requires an argument <application_address> instead of [address].

  2. In the file query_supplier.go, the CmdShowSupplier function's Use field has been updated from "show-supplier [address]" to "show-supplier <supplier_address>". This change modifies the command usage pattern by replacing the optional [address] argument with the required <supplier_address> argument.

  3. The file "go.sum" includes the addition of new dependencies: "github.com/gogo/googleapis v0.0.0-20180223154316-0cd9801be74a", "github.com/gogo/status v1.1.1", "google.golang.org/genproto v0.0.0-20180518175338-11a468237815", and "google.golang.org/grpc v1.12.0".

  4. The file session.go in the testutil/keeper package has the following changes:

    • Variables TestServiceId1 and TestServiceId2 have been updated, and new variables TestServiceId11 and TestServiceId22 have been added.
    • Variable TestServiceId12 has been added.
    • Variables TestApp1Address and TestApp2Address have been updated.
    • Application configurations for TestApp1 and TestApp2 have been updated.
    • Supplier configurations for TestSupplier1 and TestSupplier2 have been updated.
  5. In the file session_hydrator.go, the following changes are made:

    • An import for github.com/pokt-network/poktroll/x/shared/helpers has been added.
    • Error messages are updated to use specific error types such as types.ErrSessionHydration, types.ErrSessionAppNotFound, types.ErrSessionAppNotStakedForService, and types.ErrSessionSuppliersNotFound.
    • Additional checks and error handling are added for session hydration and supplier validation.
  6. The file diff includes the addition of the required module "github.com/gogo/status" with version v1.1.1.

  7. In the file tx_delegate_to_gateway.go, the following changes are made:

    • The command delegate-to-gateway now requires a required argument gateway_address.
    • The command description and long description have been updated to reflect the required gateway_address argument.
  8. The file Makefile has several changes, mainly updating the targets to use the poktrolld_addr helper and adding new targets.

  9. The diff in the file "service.proto" includes changes to the comments and message fields. Only the comments have been modified, whereas the field names remain the same.

  10. In the file tx_stake_gateway.go, the changes involve updates to the CmdStakeGateway function, including changes to command usage, short description, and long description. Additionally, the import statements have been reorganized.

  11. The file QueryGetSessionRequest is a new file that introduces the QueryGetSessionRequest type in the types package. It also defines the NewQueryGetSessionRequest constructor function and the ValidateBasic method for this type.

  12. In the file query_get_session.go, the changes include input validation for the req parameter and the use of the current block height if the block height is not specified. The session hydration function is called with the context and session hydrator.

  13. The file service_test.go has undergone changes related to service validation, including test case additions and the introduction of a dedicated function to validate service names.

  14. The file helpers_test.go has undergone changes related to test file setup, including additional import statements, initialization of the SDK configuration, and creation of network objects.

  15. In the file x/session/keeper/query_get_session_test.go, changes involve the addition of test cases and import statements.

  16. The changes in the file "service_test.go" suggest enhancements to test coverage and handling of more edge cases for the IsValidService function.

  17. The changes in the file helpers_test.go involve the addition and modification of import statements, declaration of a variable to avoid unused import error, initialization of the SDK configuration, and test file setup.

  18. Based on the file diff, changes were made to the New function, involving the removal and addition of code related to the SessionKeeper.

  19. The changes in the file "tx_submit_proof.go" involve import statements, usage message updates, and code arrangement.

  20. The file network.go has been modified to add a new item to the DefaultApplicationModuleGenesisState function and to include a comment suggesting evaluation of the `nullify.Fill

Issue

Type of change

Select one or more:

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

Testing

  • Run all unit tests: make go_develop_and_test
  • Verify Localnet manually: See the instructions [here](TODO: add link to instructions)

Sanity Checklist

  • I have tested my changes using the available tooling
  • I have performed a self-review of my own code
  • I have commented my code, updated documentation and left TODOs throughout the codebase

    - Update the CLI code flow to be able to call `GetSession`
    - Add make targets to trigger `GetSession`
    - Replace `[]` with `<>` in all CLI functions to differentiate required vs optional params
    - Made a couple changes to `SessionHydrator` (and added tests) to account additional error cases

    https://github.com/pokt-network/poktroll/assets/1892194/56425906-d06f-4c41-b8a5-d210d8a450cd

    ---

    Co-authored-by: Bryan White <[email protected]>
    Co-authored-by: Daniel Olshansky <[email protected]>
    Co-authored-by: Dima Kniazev <[email protected]>
@Olshansk Olshansk self-assigned this Nov 8, 2023
@Olshansk Olshansk added the session Changes related to Session management label Nov 8, 2023
@Olshansk Olshansk added this to the Shannon TestNet milestone Nov 8, 2023
@Olshansk
Copy link
Member Author

Olshansk commented Nov 8, 2023

@bryanchriswhite @red-0ne I merged #123 not into main and simply ran git cherry-pick bb59c24e7dfab75ab20418a4fe11d72faab82080 to create this branch. Please approve :)

Copy link
Contributor

@red-0ne red-0ne left a comment

Choose a reason for hiding this comment

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

Approved as per #123 approval.

@Olshansk Olshansk merged commit 8700722 into main Nov 8, 2023
7 checks passed
@Olshansk Olshansk deleted the issues/15/implement_get_session_cli_attempt_2 branch November 8, 2023 21:01
okdas added a commit that referenced this pull request Nov 14, 2024
- Update the CLI code flow to be able to call `GetSession`
- Add make targets to trigger `GetSession`
- Replace `[]` with `<>` in all CLI functions to differentiate required vs optional params
- Made a couple changes to `SessionHydrator` (and added tests) to account additional error cases

https://github.com/pokt-network/poktroll/assets/1892194/56425906-d06f-4c41-b8a5-d210d8a450cd

---

Co-authored-by: Bryan White <[email protected]>
Co-authored-by: Daniel Olshansky <[email protected]>
Co-authored-by: Dima Kniazev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
session Changes related to Session management
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants