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

Update README.md file #89

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Update README.md file #89

wants to merge 8 commits into from

Conversation

soumya-78
Copy link

Added detailed instructions to the README.md file to help new contributors set up their local environment for development. Also included links to relevant documentation and resources.

Soumya Ranjan added 5 commits March 5, 2023 20:16
…utors set up their local environment for development Fixes GaloyMoney#123
…utors set up their local environment for development Fixes GaloyMoney#123
…utors set up their local environment for development Fixes GaloyMoney#123
…utors set up their local environment for development Fixes GaloyMoney#123
…utors set up their local environment for development Fixes GaloyMoney#123
README.md Outdated

1. Clone the repository using git clone `https://github.com/GaloyMoney/galoy-cli.git`.
2. Install Rust in your local machine and run cargo build to build all binary and library targets of the selected packages.
3. Run cargo run command to run all tests of the Galoy CLI repository and see the usage, commands, and options available to interact.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
3. Run cargo run command to run all tests of the Galoy CLI repository and see the usage, commands, and options available to interact.
3. Run `cargo run` command to run all tests of the Galoy CLI repository and see the usage, commands, and options available to interact.

command should be within ` "brackets"

README.md Outdated
`GALOY_API=https://api.staging.galoy.io/graphql ./target/debug/galoy-client getinfo`
This command will retrieve the global values from the Galoy instance.

You can also test if the "GALOY CLI CAPTCHA SERVER" is running or not by running the following command:
Copy link
Member

Choose a reason for hiding this comment

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

this is not very clear.

README.md Outdated
3. Run cargo run command to run all tests of the Galoy CLI repository and see the usage, commands, and options available to interact.
4. Interact with the CLI yourself to become familiar with it. After building, try the following command:

`GALOY_API=https://api.staging.galoy.io/graphql ./target/debug/galoy-client getinfo`
Copy link
Member

Choose a reason for hiding this comment

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

ideally we would want to highlight that the backend can also be run locally by running https://github.com/GaloyMoney/galoy (so you don't have to fetch our staging environment). the captcha can also be deactivated with the local dev environment

Copy link
Author

Choose a reason for hiding this comment

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

So i need to remove this?

Copy link
Author

Choose a reason for hiding this comment

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

@nicolasburtey i think i should update the readme with providing the link to galoy backend(https://github.com/GaloyMoney/galoy) in place of these commands.

README.md Outdated

-a, --api <API>: Set the API URL (default: http://localhost:4002/graphql) <br/>
-d, --debug: Enable debug mode<br/>
-j, --jwt <JWT>: Set the JWT for authorization<br/>
Copy link
Contributor

@thevaibhav-dixit thevaibhav-dixit Mar 5, 2023

Choose a reason for hiding this comment

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

I think jwt is not a valid term for the access token we get when we login ? I even got error when I tired to set in access token as JWT in the cli. @nicolasburtey

Copy link
Author

Choose a reason for hiding this comment

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

I am also getting error. basically showing "jwt syntax issue".

Copy link
Member

Choose a reason for hiding this comment

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

yup long story short, there should no longer be a JWT stored, but a kratos token. it should also be passed as a Authorization header to the backend.

Copy link
Member

Choose a reason for hiding this comment

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

it probably means this command line should be update. maybe there is no code change necessary because functionally if we assumed the JWT was an opaque token (ie: we were not trying to decode it), then it should be functionally the same with the kratos token

Copy link
Contributor

@thevaibhav-dixit thevaibhav-dixit Mar 5, 2023

Choose a reason for hiding this comment

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

imho to make this functional we need to remove this https://github.com/GaloyMoney/galoy-cli/blob/main/src/main.rs#L84-L86 . Here we are trying to check for a valid JWT. I tried to this way and it worked for me. Maybe we can replace this with a simple check if the token adheres to a fix length ?

Copy link
Member

Choose a reason for hiding this comment

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

yup this should not longer work with the new way we authenticate query. this should be removed.

Copy link
Author

Choose a reason for hiding this comment

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

@nicolasburtey if we remove the part mentioned by @thevaibhav-dixit then how to authenticate, like based on the length of JWT?

Copy link
Author

Choose a reason for hiding this comment

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

And the error in checks coming due to old version of remove_dir_all that is 0.5.3 as it is supported with version above 0.8.0. but there is no crate "remove_dir_all" in cargo.toml that need to be updated. how to solve this issue can you please guide me?

@soumya-78 soumya-78 requested a review from nicolasburtey March 6, 2023 15:59
README.md Outdated
## Installation
To install Galoy CLI locally and set up a local environment:

1. Clone the repository using git clone `https://github.com/GaloyMoney/galoy-cli.git`.
Copy link

Choose a reason for hiding this comment

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

  1. Clone the /galoy-cli repository using git clone https://github.com/GaloyMoney/galoy-cli.git

r0ushann

This comment was marked as duplicate.

Copy link

@r0ushann r0ushann left a comment

Choose a reason for hiding this comment

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

Galoy-cli

Galoy CLI is a Rust-based CLI client that can interact with the Galoy backend using GraphQL queries and mutations.

Rust Installation and cargo commands

go through the installation guide at https://www.rust-lang.org/learn/get-started

galoy-cli Installation

for getting started with the installation and running galoy-cli locally , set up a local environment first.

  1. Clone galoy-cli repository using git clone https://github.com/GaloyMoney/galoy-cli.git
  2. Install Rust in your local machine and run cargo build to build all binary and library targets of the selected packages.
  3. Run cargo run command to run your project and cargo test to run all the tests of the Galoy CLI repository and see the usage, commands, and options available to interact.
  4. Interact with the CLI yourself to become familiar with it.

The Galoy backend can also be run locally by cloning the repository from https://github.com/GaloyMoney/galoy. Running the backend locally allows you to test and develop features without needing to fetch data from our staging environment. Additionally, the CAPTCHA feature can be deactivated in the local development environment.

@sandipndev sandipndev force-pushed the main branch 4 times, most recently from 64ab5f2 to 330afe0 Compare August 3, 2023 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants