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

feat: Enable Metadata Table Writes #659

Merged
merged 12 commits into from
Apr 17, 2024
Merged

feat: Enable Metadata Table Writes #659

merged 12 commits into from
Apr 17, 2024

Conversation

darunrs
Copy link
Collaborator

@darunrs darunrs commented Apr 12, 2024

Enable writes of Status and Last Processed Block Height to Metadata table. Reorganizes provisioning to ensure writing of PROVISIONING status. Ensures IndexerMeta is available for writing error logs.

await this.deps.indexerMeta?.setStatus(status);
}

async setStoppedStatus (): Promise<void> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Specifically for use by StreamHandler to set STOPPED status. It's possible the connection parameters/table aren't prepared so I try getting those first.


it('works', async () => {
const hasuraClient = new HasuraClient({}, {
beforeEach(async () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved declaration of hasura, pgClient, and provisioner to beforeEach since so far all tests use the same config.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe for a different PR, but we should look in to setting up isolated containers/clients/config. Right now they are shared across tests which could lead to some issues.

Right now, the test context db test probably only works since it has a different function name, which therefore forces it to provision. But if we used the same function name as the other tests it would fail.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah it did fail originally, so I changed the function name haha. And yeah I agree. Definitely worth thinking about soon.

@darunrs
Copy link
Collaborator Author

darunrs commented Apr 12, 2024

Tested FAILING indexers and had worker crash to verify setting of STOPPED status. All seems to be working. Setting of block height also works.

@darunrs darunrs marked this pull request as ready for review April 12, 2024 01:32
@darunrs darunrs requested a review from a team as a code owner April 12, 2024 01:32
Base automatically changed from provision-metadata-table to main April 12, 2024 18:09
@darunrs darunrs linked an issue Apr 16, 2024 that may be closed by this pull request
@darunrs
Copy link
Collaborator Author

darunrs commented Apr 16, 2024

Cannot be merged without #660

@@ -89,14 +90,16 @@ export default class Indexer {

try {
if (!await this.deps.provisioner.fetchUserApiProvisioningStatus(this.indexerConfig)) {
// TODO: Remove call as setting PROVISIONING status is impossible with new table
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we removing the PROVISIONING status?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct. It's no longer possible to set this status as we cannot guarantee the existence of the table. Most of the time, it won't be.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No longer removing this. Setting of provisioning is now performed in Provisioner.

runner/src/indexer/indexer.ts Outdated Show resolved Hide resolved

it('works', async () => {
const hasuraClient = new HasuraClient({}, {
beforeEach(async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe for a different PR, but we should look in to setting up isolated containers/clients/config. Right now they are shared across tests which could lead to some issues.

Right now, the test context db test probably only works since it has a different function name, which therefore forces it to provision. But if we used the same function name as the other tests it would fail.

runner/tests/integration.test.ts Outdated Show resolved Hide resolved
@darunrs darunrs force-pushed the enable-metadata-writes branch 2 times, most recently from 00c8b8c to 9c142fd Compare April 16, 2024 21:35
@darunrs darunrs force-pushed the enable-metadata-writes branch from 0a84c0c to 965e657 Compare April 17, 2024 19:28
@darunrs darunrs merged commit 492d95c into main Apr 17, 2024
3 checks passed
@darunrs darunrs deleted the enable-metadata-writes branch April 17, 2024 23:52
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.

Write statuses to metadata table and change the UI
2 participants