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

Use new Metadata Table for status and block height #676

Merged
merged 5 commits into from
Apr 22, 2024

Conversation

darunrs
Copy link
Collaborator

@darunrs darunrs commented Apr 19, 2024

Frontend now queries the new metadata table instead of the old one.
image

Indexers which have not successfully run since the introduction of the new logs table will not have a last processed block height since they never processed a block successfully. So, I opted to set it as N/A and leave a tooltip on hover which says why its N/A.
image

@darunrs
Copy link
Collaborator Author

darunrs commented Apr 19, 2024

Query needs to be renamed after creating new metadata table. Existing metadata and logs tables are prepended with double underscores, which are reserved by graphQL. The tables need to be renamed. Will merge this when the new tables are prepared.

@darunrs darunrs marked this pull request as ready for review April 19, 2024 01:10
@darunrs darunrs requested a review from a team as a code owner April 19, 2024 01:10
Copy link
Collaborator

@morgsmccauley morgsmccauley left a comment

Choose a reason for hiding this comment

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

Just one comment

current_block_height
current_historical_block_height
const Status = ({ accountId, functionName, latestHeight }) => {
const hasuraRole = accountId.replace(/[^a-zA-Z0-9]/g, '_'); // TODO: Support accounts that start with a number
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we support this TODO now? Implicit accounts are likely to start with numbers

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure I can add the fix in for it now. I'll do so partially for now. The second part of this is quoting database names from now on. If a database or schema is created with a capital letter, it will fail if a similar name without capital letters was already created. For example, if you tried to provision an indexer as Morgs.near, the DB will be created correctly, and a schema will too, albeit with lowercase name. But the tables will fail to create. I'll support that separately. It needs more strict testing to ensure backwards compatibility.

@darunrs darunrs force-pushed the use-metadata-table-frontend branch from d069c4c to 67cfdfa Compare April 22, 2024 18:21
@@ -12,9 +12,9 @@ First, download the bos-loader cli by following this guide [here](https://docs.n
From the root of QueryAPI Frontend repo, run the following command

```bash
yarn serve:widgets:local // for running local enviornment
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since we removed the yarn file from Runner, we should do the same for frontend too. I checked and we do not use any yarn commands in the deployment of frontend in Terraform. Deleting yarn.lock for the same reason.

@@ -54,11 +54,12 @@
"@opentelemetry/api": "^1.8.0",
"@opentelemetry/exporter-zipkin": "^1.22.0",
"@opentelemetry/resources": "^1.22.0",
"@opentelemetry/sdk-node": "^0.49.1",
"@opentelemetry/sdk-node": "^0.50.0",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Needed to bump version as I was getting some type mismatch errors in IDE.

"@opentelemetry/sdk-trace-base": "^1.22.0",
"@opentelemetry/sdk-trace-node": "^1.22.0",
"@opentelemetry/semantic-conventions": "^1.22.0",
"express": "^4.18.2",
"graphql": "^16.8.1",
Copy link
Collaborator Author

@darunrs darunrs Apr 22, 2024

Choose a reason for hiding this comment

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

Apparently we were using this package as a dependency in import { gql, GraphQLClient } from 'graphql-request'; in integration tests, but it wasn't automatically added through install.

Comment on lines +7 to +8
.vscode/
*/yarn.lock
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are mainly for convenience. .vscode folder is necessary for the IDE to work right, and I'm ignoring yarn.lock files to prevent publishing of them again in the future.

overlay={<Tooltip> {
attributeMap.get("LAST_PROCESSED_BLOCK_HEIGHT")
? `Current Block Height of Near is ${latestHeight}. Your indexer has a gap of ${latestHeight - attributeMap.get("LAST_PROCESSED_BLOCK_HEIGHT")} Blocks`
: 'Indexer needs to run successfully to update block height' } </Tooltip>}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is possible now for failing indexers to not have a last processed block as they haven't successfully processed a block since the new table was rolled out.

We only set the height after a successful run. I have a custom message for this case.

@darunrs darunrs merged commit b8f9db6 into main Apr 22, 2024
3 checks passed
@darunrs darunrs deleted the use-metadata-table-frontend branch April 22, 2024 19:32
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