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: graph-node-status-endpoint for DeploymentDetail status_url #91

Merged
merged 1 commit into from
Nov 10, 2023

Conversation

hopeyen
Copy link
Collaborator

@hopeyen hopeyen commented Nov 8, 2023

Indexer service takes in

graph_node_query_endpoint // commonly 'http://localhost:8000'
graph_node_status_endpoint // commonly 'http://localhost:8030/graphql'

DeploymentDetails::for_graph_node currently generalized graph_node_base_url for both

  • status_url by appending /status
  • query_url by appending /subgraphs/id/:id

I made updates to use the configured --graph-node-status-endpoint for status_url instead of graph_node_query_endpoint.

Resolves #89

@hopeyen hopeyen requested a review from Theodus November 8, 2023 18:53
@hopeyen hopeyen self-assigned this Nov 8, 2023
@hopeyen hopeyen added size:small Small p1 High priority type:bug Something isn't working labels Nov 8, 2023
Theodus
Theodus previously approved these changes Nov 8, 2023
Copy link
Member

@Theodus Theodus left a comment

Choose a reason for hiding this comment

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

Thanks! I've confirmed this is now working as expected on the local-network

Copy link
Contributor

@aasseman aasseman left a comment

Choose a reason for hiding this comment

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

There may be an issue with the tests though. I restarted the job to see if it was a fluke.

@aasseman
Copy link
Contributor

aasseman commented Nov 8, 2023

There's definitely an issue with the tests:

test subgraph_client::client::test::test_uses_local_deployment_if_healthy_and_synced has been running for over 60 seconds
test subgraph_client::client::test::test_uses_query_url_if_local_deployment_is_not_synced has been running for over 60 seconds

@hopeyen could you try running the tests locally and see what happens?

@hopeyen hopeyen force-pushed the hope/dpmt-detail-for-graph-node branch from cf3d229 to 87f4651 Compare November 9, 2023 11:57
Copy link
Contributor

github-actions bot commented Nov 9, 2023

Pull Request Test Coverage Report for Build 6811417087

  • 71 of 73 (97.26%) changed or added relevant lines in 2 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.3%) to 50.447%

Changes Missing Coverage Covered Lines Changed/Added Lines %
service/src/main.rs 0 2 0.0%
Files with Coverage Reduction New Missed Lines %
common/src/subgraph_client/client.rs 3 94.4%
Totals Coverage Status
Change from base Build 6801998160: 0.3%
Covered Lines: 1411
Relevant Lines: 2797

💛 - Coveralls

@hopeyen
Copy link
Collaborator Author

hopeyen commented Nov 9, 2023

@aasseman thanks for raising! I fixed the tests in 87f4651

@hopeyen hopeyen requested a review from aasseman November 9, 2023 12:15
@hopeyen hopeyen merged commit 7e3595b into main Nov 10, 2023
5 checks passed
@hopeyen hopeyen deleted the hope/dpmt-detail-for-graph-node branch November 10, 2023 04:46
@hopeyen hopeyen restored the hope/dpmt-detail-for-graph-node branch November 10, 2023 04:46
@hopeyen hopeyen deleted the hope/dpmt-detail-for-graph-node branch November 10, 2023 04:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p1 High priority size:small Small type:bug Something isn't working
Projects
Status: 🚗 Merged
Development

Successfully merging this pull request may close these issues.

incorrect subgraph indexing status check
3 participants