Skip to content

Conversation

@dbeal-eth
Copy link
Contributor

in order to use the same subgraph for multiple neworks (ex. mainnet, kovan, mainnet-ovm, etc.), it is currently
necessary to construct a fresh subgraph manifest for each network and supply them individually.

this PR is a slight modification to the CLI in order to resolve the manifest definition from a JS file. as long
as the javascript resolves its module.exports to the JSON of the manifest, custom code can be used to generate
the appropriate manifest

this should not break any existing compatibility with YAML or JSON loading.

in order to use the same subgraph for multiple neworks (ex. mainnet, kovan, mainnet-ovm, etc.), it is currently
necessary to construct a fresh subgraph manifest for each network and supply them individually.

this PR is a slight modification to the CLI in order to resolve the manifest definition from a JS file. as long
as the javascript resolves its `module.exports` to the JSON of the manifest, custom code can be used to generate
the appropriate manifest

this should not break any existing compatibility with YAML or JSON loading.
@dbeal-eth
Copy link
Contributor Author

/cc @dvd-schwrtz

Copy link
Collaborator

@fordN fordN left a comment

Choose a reason for hiding this comment

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

Left a comment about loading the yaml module which looks like the culprit of the failed builds. I'd also love to see a few validation tests for JS manifests loaded using this new manifest file type support. (https://github.com/graphprotocol/graph-cli/tree/master/tests/cli/validation)

* fix missing yaml module not included (oops)
* better error outputs
@dbeal-eth
Copy link
Contributor Author

@fordN regarding validation tests, I am scared of adding such a test since the errors for this change would be complteely generated from Node.js, which might evolve its error format over time. Let me know what kind of tests you want to see.

also, I added a change to make the initial load of the module more intelligent, and instead it will load based on the file extension so taht the require error is not shadowed.

@dbeal-eth
Copy link
Contributor Author

@fordN JSYK, a test has been successfully completed on my local graph node to verify that manifests using this method are deployed and operating as expected.

dbeal-eth added a commit to dbeal-eth/synthetix-subgraph that referenced this pull request Apr 22, 2021
using new feature to be implemented in branched graph-cli graphprotocol/graph-tooling#642

this new manifest refactor has the following benefits:

* multi-network compatible by generating based on environment
* no more hardcoding addresses. It reads both the address and the startBlock from the `versions` manifest of synthetix
* no more hardcoding startBlocks
* full flexibility for overrides, manual values, etc. due to full javascript generation
* no network calls, completely synchronous (i.e. its still fast)
Copy link
Collaborator

@fordN fordN left a comment

Choose a reason for hiding this comment

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

Looking good! 👍

I've tested the update as well by using deploying a test .js manifest subgraph to the hosted-service.

@leoyvens leoyvens merged commit 14fb300 into graphprotocol:master Apr 23, 2021
dvd-schwrtz pushed a commit to Synthetixio/synthetix-subgraph that referenced this pull request May 12, 2021
* add dynamic generation of manifests

using new feature to be implemented in branched graph-cli graphprotocol/graph-tooling#642

this new manifest refactor has the following benefits:

* multi-network compatible by generating based on environment
* no more hardcoding addresses. It reads both the address and the startBlock from the `versions` manifest of synthetix
* no more hardcoding startBlocks
* full flexibility for overrides, manual values, etc. due to full javascript generation
* no network calls, completely synchronous (i.e. its still fast)

* most of the charts moved over

just a few things to work out now, and lots of testing

note: use `codegen` to make the new codegen

* add in small build scripts, fixes

* add code for loans and shorts and start working on build error messages

* fix build issues for rates and add env variable to build command

* fixes

* update synthetix js file with new contract names to add to 'yaml' input

* try to fix latestrates issue, but it wont budge

* remove hardcoded values

* fixes for adding aggregator proxy

* add prettier as a pre-commit hook

* start inverse rate impl

* add missing

* try to fix chainlink contracts

* more debug

* fix for GnosisSafe handler of chainlink multisig to auto-refresh aggregators

* fixes

* fixes

* prettier

* fixes for comments

* doc fixes, rates contract available now

* fixes for BigDecimal usage

* add upgrade block guards
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.

3 participants