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/objectarium instance #82

Merged
merged 7 commits into from
Sep 7, 2023
Merged

Conversation

ErikssonJoakim
Copy link
Contributor

@ErikssonJoakim ErikssonJoakim commented Sep 5, 2023

This PR finishes the following issue by implementing a objectarium handler for indexing the initialization of objectarium smart contracts:

As of date there seems to be a problem filtering the code id through the manifest so the contract filtering is implemented in the handler.

objectaria query including config and limits
{
  "data": {
    "query": {
      "objectaria": {
        "id": "okp41jg5y3m2u9ac50g5jsd7mksh3y9nvp3pyuf5fj8entsk9tv2tq4gsv9r25d",
        "owner": "okp41zn4x0geu5eemk7ul05ld7n9e4kzs7cxx4al3fs",
        "config": {
          "hashAlgorithm": "sha224",
          "acceptedCompressionAlgorithms": [
            "snappy"
          ]
        },
        "limits": {
          "maxObjects": "20",
          "maxTotalSize": "1000",
          "maxObjectPins": "1",
          "maxObjectSize": "50"
        },
        "objectariumObjects": {
          "nodes": []
        },
        "messages": {
          "nodes": [
            {
              "block": {
                "height": "4004814"
              }
            }
          ]
        }
      }
    }
  }
}

@ErikssonJoakim ErikssonJoakim self-assigned this Sep 5, 2023
@ErikssonJoakim ErikssonJoakim marked this pull request as ready for review September 5, 2023 08:40
schema.graphql Show resolved Hide resolved
schema.graphql Show resolved Hide resolved
schema.graphql Show resolved Hide resolved
src/mappings/objectariumHandlers.ts Show resolved Hide resolved
Copy link
Member

@ccamel ccamel left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

Copy link
Contributor

@fredericvilcot fredericvilcot left a comment

Choose a reason for hiding this comment

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

As a general remark, errors are handled nowhere in code and I see no relevant test suite.
Are there plans to implement it in the near future?

src/mappings/helper.ts Show resolved Hide resolved
src/mappings/objectariumHandlers.ts Outdated Show resolved Hide resolved
@ccamel
Copy link
Member

ccamel commented Sep 6, 2023

As a general remark, errors are handled nowhere in code and I see no relevant test suite. Are there plans to implement it in the near future?

That's a good question. AFAIU, the code for indexing (i.e. mapping) is ran in a virtual machine (required for decentralisation) driving the process. That's why the best approach here is the "let's propagate" and "fail fast" pattern, and just let the node runner do what it takes to process properly.

Test suites are considered, but it's currently difficult to achieve. You can get more info here: https://academy.subquery.network/build/testing.html

@fredericvilcot
Copy link
Contributor

As a general remark, errors are handled nowhere in code and I see no relevant test suite. Are there plans to implement it in the near future?

That's a good question. AFAIU, the code for indexing (i.e. mapping) is ran in a virtual machine (required for decentralisation) driving the process. That's why the best approach here is the "let's propagate" and "fail fast" pattern, and just let the node runner do what it takes to process properly.

Test suites are considered, but it's currently difficult to achieve. You can get more info here: https://academy.subquery.network/build/testing.html

@ccamel
Thanks for these explanations, I see now the point much clearer.
I agree that failing fast is a great move but it must not justify the non-handing of errors in the whole codebase.
We will especially lack the immediate context of the error, and lose the execution stack trace, IMHO. And thus even more if the error is not well handled in the library..

Copy link
Member

@amimart amimart left a comment

Choose a reason for hiding this comment

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

LGTM!

Following remarks about error handling, I agree with the failing fast mechanism and to go further, I think this is something that should be took into account at the operation level and be monitored :)

@ErikssonJoakim
Copy link
Contributor Author

@amimart for the monitoring SubQuery does offer a management service where it's possible to receive alerts on the node health regarding indexer stalling, service health and deployment status

Copy link
Contributor

@fredericvilcot fredericvilcot left a comment

Choose a reason for hiding this comment

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

LGTM

@ErikssonJoakim ErikssonJoakim merged commit dfd6370 into main Sep 7, 2023
13 of 14 checks passed
@ErikssonJoakim ErikssonJoakim deleted the feat/objectarium-instance branch September 7, 2023 14:40
@bot-anik
Copy link
Member

🎉 This PR is included in version 1.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants