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

Rosetta cli changes according to new indexer #64

Merged
merged 3 commits into from
Dec 21, 2022

Conversation

Haider-Ali-DS
Copy link
Contributor

Description

Changed implementation for rosetta-cli according to new indexer implementation

Type of change

Please delete options that are not relevant.

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Tests

Describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration

  • Manual testing

Code review prechecks:

  • Code follows the style guidelines of this project
  • Code has been self-reviewed
  • Inline comments have been added for each method
  • I have made corresponding changes to the documentation
  • Code changes introduces no new problems or warnings
  • Test cases have been added
  • Dependent changes have been merged and published in downstream modules

README.md Outdated

rosetta-cli --url=http://localhost:8081 --chain=eth search --indexer-url=http://localhost:8084 --type=Transfer --success=true

rosetta-cli --url=http://localhost:8082 --chain=dot search --indexer-url=http://localhost:8085 --type=Transfer --success=true
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the url arg needed for here? or why do we need a new indexer-url arg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

since the indexer will be running on a separate port than rosetta so i added the indexer-url. Should we add this in config? Url arg is for server default will connect to rosetta.analog.one

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, it will be running on a separate port. I think it would be better to forward the non search requests, it's a bit messy at the moment.

but my main question was, doesn't this work?

rosetta-cli --url=http://localhost:8085 --chain=dot search --type=Transfer --success=true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah right,

Copy link
Contributor Author

@Haider-Ali-DS Haider-Ali-DS Dec 21, 2022

Choose a reason for hiding this comment

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

so i removed url arg from the readme file but would need that additional param in args to differentiate between rosetta-server url and rosetta-indexer url since we need both at somepoint.

@dvc94ch dvc94ch merged commit d988697 into master Dec 21, 2022
@dvc94ch dvc94ch deleted the rosetta-cli-indexer-fix branch December 21, 2022 17:09
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.

2 participants