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

fix(rust): improve ockam relay list display when local state is empty #6204

Conversation

k1nho
Copy link
Contributor

@k1nho k1nho commented Sep 30, 2023

Current behavior

Fixes #6144

$ ockam relay list returns an immediate error Unable to find node name default when the local state is empty, after running a command like $ ockam reset -y.

Proposed changes

This PR improves the user experience by handling the error when the local state is empty and displaying that there are no relays. Running $ ockam relay list on an empty local state will now yield the following.

Screenshot 2023-09-30 at 3 49 45 PM

Checks

  • All commits in this Pull Request are signed and Verified by Github.
  • All commits in this Pull Request follow the Ockam commit message convention.
  • There are no Merge commits in this Pull Request. Ockam repo maintains a linear commit history. We merge Pull Requests by rebasing them onto the develop branch. Rebasing to the latest develop branch and force pushing to your Pull Request branch is okay.
  • I have read and accept the Ockam Community Code of Conduct.
  • I have read and accepted the Ockam Contributor License Agreement by adding my Git/Github details in a row at the end of the CONTRIBUTORS.csv file in a separate pull request to the build-trust/ockam repository. The easiest way to do this is to edit the CONTRIBUTORS.csv file in the github web UI and create a separate Pull Request, this will mark the commit as verified.

@k1nho k1nho requested a review from a team as a code owner September 30, 2023 19:54
@k1nho k1nho mentioned this pull request Sep 30, 2023
5 tasks
Copy link
Member

@adrianbenavides adrianbenavides left a comment

Choose a reason for hiding this comment

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

Hi @k1nho, thanks for submitting this PR! Your solution is valid, but there is a way to achieve the same result without adding extra code to the command impementation: take a look at the tcp-inlet list as an example. By running the initialize_node_if_default function before executing the run_impl you make sure at least the default node is up. The command will be able to handle the empty response, returning somthing like:

$ ockam relay list

       ┌────────────────────────────────────┐
       │       Relays on Node default       │
       └────────────────────────────────────┘

     ! No Relays found on node default.

@k1nho k1nho force-pushed the k1nho/improve-ockam-relay-list-when-empty branch from bbb81e6 to 01c2627 Compare October 2, 2023 16:00
@k1nho
Copy link
Contributor Author

k1nho commented Oct 2, 2023

Hi @adrianbenavides, thanks for the feedback! good to know there is a useful helper for this case.

@mrinalwadhwa mrinalwadhwa added the hacktoberfest Apply to issues you want contributors to help with label Oct 2, 2023
@adrianbenavides adrianbenavides force-pushed the k1nho/improve-ockam-relay-list-when-empty branch from 01c2627 to 07c51d1 Compare October 3, 2023 10:09
@adrianbenavides adrianbenavides added the HACKTOBERFEST-ACCEPTED To be used when a PR is ready to merge or after it's merged label Oct 3, 2023
@adrianbenavides adrianbenavides force-pushed the k1nho/improve-ockam-relay-list-when-empty branch from 07c51d1 to 8b60a78 Compare October 3, 2023 11:45
@adrianbenavides adrianbenavides force-pushed the k1nho/improve-ockam-relay-list-when-empty branch from 8b60a78 to e2ef5d9 Compare October 3, 2023 11:50
@adrianbenavides adrianbenavides self-requested a review October 3, 2023 11:55
Copy link
Member

@adrianbenavides adrianbenavides left a comment

Choose a reason for hiding this comment

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

Thanks @k1nho for your contribution 🙏
It will get merged ASAP 🚀

@adrianbenavides adrianbenavides added this pull request to the merge queue Oct 3, 2023
Merged via the queue into build-trust:develop with commit 7ead1d0 Oct 3, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest Apply to issues you want contributors to help with HACKTOBERFEST-ACCEPTED To be used when a PR is ready to merge or after it's merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve ockam relay list when there are no relays (user is not enrolled)
3 participants