-
Notifications
You must be signed in to change notification settings - Fork 33
Documentation updates #94
Changes from 8 commits
b96f6c4
ba81766
e1a0d89
f7d520c
436d9b9
a49f5d7
5d1ba59
ade1429
7d1b334
8dc9ddf
bd3e841
fa03716
829a581
7ddf728
eba5244
4580f0f
481988c
d947c8f
622ea44
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,23 @@ | ||
# Contribution guidelines | ||
|
||
Contributions are welcome! In addition to core contributions, developers are encouraged to build their own custom transformers which | ||
Contributions are welcome! Please open an Issues or Pull Request for any changes. | ||
|
||
In addition to core contributions, developers are encouraged to build their own custom transformers which | ||
can be run together with other custom transformers using the [composeAndExeucte](../../staging/documentation/composeAndExecute.md) command. | ||
|
||
## Pull Requests | ||
- `go fmt` is run as part of `make test` and `make integrationtest`, please make sure to check in the format changes. | ||
- Ensure that new code is well tested, including integration testing if applicable. | ||
- Make sure the build is passing. | ||
- Update the README or any [documentation files](./) as necessary. If editing the Readme, please | ||
conform to the | ||
[standard-readme specification](https://github.com/RichardLitt/standard-readme). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know if we're really conforming to the standard readme spec? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah you are right we aren't. I think the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call, I think in the standard readme spec it includes updated here: 69b4431 |
||
- You may merge a Pull Request once you have an approval from core developer. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the general workflow that we've been following, but I wonder if we should rethink this in the future when additional contributors come onto the project so that we can continue to sanely add new features. Do folks think that we should require 2 approvals? Only allow "core developer" merging permissions? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should also mention to tag a few of us as Reviewers on any new Pull Request? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely on board with requiring 2 approves and only allowing core members to merge PRs. I think (hope) the latter is already the case There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also onboard with with requiring 2 approves and restricting merges to core. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 cool, I'll update the language in this document, and change the repo settings to require 2 reviews before allowing a merge. |
||
|
||
## Creating a new migration file | ||
1. `make new_migration NAME=add_columnA_to_table1` | ||
- This will create a new timestamped migration file in `db/migrations` | ||
1. Write the migration code in the created file, under the respective `goose` pragma | ||
- Goose automatically runs each migration in a transaction; don't add `BEGIN` and `COMMIT` statements. | ||
1. Core migrations should be committed in their `goose fix`ed form. | ||
1. Core migrations should be committed in their `goose fix`ed form. To do this, run `make version_migrations` which | ||
converts timestamped migrations to migrations versioned by an incremented integer. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,43 +1,78 @@ | ||
# composeAndExecute | ||
The `composeAndExecute` command is used to compose and execute over an arbitrary set of custom transformers. | ||
This is accomplished by generating a Go pluggin which allows the `vulcanizedb` binary to link to external transformers, so | ||
long as they abide by one of the standard [interfaces](../staging/libraries/shared/transformer). | ||
|
||
Additionally, there are separate `compose` and `execute` commands to allow pre-building and linking to a pre-built .so file. | ||
|
||
**NOTE:** | ||
1. It is necessary that the .so file was built with the same exact dependencies that are present in the execution environment, | ||
i.e. we need to `compose` and `execute` the plugin .so file with the same exact version of vulcanizeDB. | ||
1. The plugin migrations are run during the plugin's composition. As such, if `execute` is used to run a prebuilt .so in a different | ||
environment than the one it was composed in then the migrations for that plugin will first need to be manually ran against that environment's Postgres database. | ||
|
||
These commands require Go 1.11+ and use [Go plugins](https://golang.org/pkg/plugin/) which only work on Unix-based systems. | ||
There is also an ongoing [conflict](https://github.com/golang/go/issues/20481) between Go plugins and the use vendored dependencies which | ||
imposes certain limitations on how the plugins are built. | ||
|
||
## Commands | ||
The `compose` and `composeAndExecute` commands assume you are in the vulcanizdb directory located at your system's `$GOPATH`, | ||
and that all of the transformer repositories for building the plugin are present at their `$GOPATH` directories. | ||
|
||
The `execute` command does not require the plugin transformer dependencies be located in their | ||
`$GOPATH` directories, instead it expects a prebuilt .so file (of the name specified in the config file) | ||
to be in `$GOPATH/src/github.com/vulcanize/vulcanizedb/plugins/` and, as noted above, also expects the plugin | ||
db migrations to have already been ran against the database. | ||
|
||
compose: | ||
|
||
`./vulcanizedb compose --config=./environments/config_name.toml` | ||
|
||
execute: | ||
|
||
`./vulcanizedb execute --config=./environments/config_name.toml` | ||
|
||
composeAndExecute: | ||
|
||
`./vulcanizedb composeAndExecute --config=./environments/config_name.toml` | ||
|
||
## Flags | ||
|
||
# Custom Transformers | ||
When the capabilities of the generic `contractWatcher` are not sufficient, custom transformers tailored to a specific | ||
purpose can be leveraged. | ||
|
||
Individual custom transformers can be composed together from any number of external repositories and executed as a | ||
single process using the `compose` and `execute` commands or the `composeAndExecute` command. This is accomplished by | ||
generating a Go plugin which allows the `vulcanizedb` binary to link to the external transformers, so long as they | ||
abide by one of the standard [interfaces](../staging/libraries/shared/transformer). | ||
|
||
## Writing custom transformers | ||
For help with writing different types of custom transformers please see below: | ||
|
||
Storage Transformers | ||
* [Guide](../../staging/libraries/shared/factories/storage/README.md) | ||
* [Example](../../staging/libraries/shared/factories/storage/EXAMPLE.md) | ||
|
||
Event Transformers | ||
* [Guide](../../staging/libraries/shared/factories/event/README.md) | ||
* [Example 1](https://github.com/vulcanize/ens_transformers/tree/master/transformers/registar) | ||
* [Example 2](https://github.com/vulcanize/ens_transformers/tree/master/transformers/registry) | ||
* [Example 3](https://github.com/vulcanize/ens_transformers/tree/master/transformers/resolver) | ||
|
||
Contract Transformers | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps it would be useful to include brief description of what each transformer does and how they differ from one another? Idk, maybe the links are sufficient, was just thinking something like:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the last one, maybe something like
? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 7ddf728 |
||
* [Example 1](https://github.com/vulcanize/account_transformers) | ||
* [Example 2](https://github.com/vulcanize/ens_transformers/tree/master/transformers/domain_records) | ||
|
||
## Preparing custom transformers to work as part of a plugin | ||
To plug in an external transformer we need to: | ||
|
||
1. Create a package that exports a variable `TransformerInitializer`, `StorageTransformerInitializer`, or `ContractTransformerInitializer` that are of type [TransformerInitializer](../staging/libraries/shared/transformer/event_transformer.go#L33) | ||
or [StorageTransformerInitializer](../../staging/libraries/shared/transformer/storage_transformer.go#L31), | ||
or [ContractTransformerInitializer](../../staging/libraries/shared/transformer/contract_transformer.go#L31), respectively | ||
2. Design the transformers to work in the context of their [event](../staging/libraries/shared/watcher/event_watcher.go#L83), | ||
[storage](../../staging/libraries/shared/watcher/storage_watcher.go#L53), | ||
or [contract](../../staging/libraries/shared/watcher/contract_watcher.go#L68) watcher execution modes | ||
3. Create db migrations to run against vulcanizeDB so that we can store the transformer output | ||
* Do not `goose fix` the transformer migrations, this is to ensure they are always ran after the core vulcanizedb migrations which are kept in their fixed form | ||
* Specify migration locations for each transformer in the config with the `exporter.transformer.migrations` fields | ||
* If the base vDB migrations occupy this path as well, they need to be in their `goose fix`ed form | ||
as they are [here](../../staging/db/migrations) | ||
|
||
To update a plugin repository with changes to the core vulcanizedb repository, run `dep ensure` to update its dependencies. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This section is super hard. I find it tricky to follow, but also don't know how to simplify without breaking out new docs. I guess I'm leaning toward just including a pretty general statement along the lines of "custom transformers can be developed as plugins and contained in separate repos. For details on building a plugin that's compatible with VulcanizeDB, see docs/building-transformer-plugins.md`. And then I feel like in that other doc, it would be helpful to explain -
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with this sentiment, and I can take the lead on cleaning this up since this my doing! Although if we extract this elsewhere I am wondering if it might make sense to include this information in the guides for writing transformers? Points 1 and 2 would fit well in their transformer's respective guides. For the third point, the config organization is expanded on further down in the config section, but I agree it should probably be lifted into a separate doc and overall needs more clarity. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would be great if you could take the lead on reworking this @i-norden, let me know if you need someone to bounce ideas off, I'd be happy to help! |
||
|
||
## Building and Running Custom Transformers | ||
### Commands | ||
* The `compose`, `execute`, `composeAndExecute` commands require Go 1.11+ and use [Go plugins](https://golang | ||
.org/pkg/plugin/) which only work on Unix-based systems. | ||
|
||
* There is an ongoing [conflict](https://github.com/golang/go/issues/20481) between Go plugins and the use vendored | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/use vendored/use of vendored |
||
dependencies which imposes certain limitations on how the plugins are built. | ||
|
||
* Separate `compose` and `execute` commands allow pre-building and linking to a pre-built .so file. So, if | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it'd be helpful to give a brief overview of |
||
these are run independently, instead of using `composeAndExecute`, a couple of things need to be considered: | ||
* It is necessary that the .so file was built with the same exact dependencies that are present in the execution | ||
environment, i.e. we need to `compose` and `execute` the plugin .so file with the same exact version of vulcanizeDB. | ||
* The plugin migrations are run during the plugin's composition. As such, if `execute` is used to run a prebuilt .so | ||
in a different environment than the one it was composed in then the migrations for that plugin will first need to | ||
be manually ran against that environment's Postgres database. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is minor but I think you could also load the plugin's schema to get around this, and that might be a more simple process if you're really interested in running a prebuilt .so file There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated it in b3019c9, let me know what you think! |
||
|
||
* The `compose` and `composeAndExecute` commands assume you are in the vulcanizdb directory located at your system's | ||
`$GOPATH`, and that all of the transformer repositories for building the plugin are present at their `$GOPATH` directories. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if "all of the transformer repositories for building the plugin are present" could be simplified to "the plugin is present" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That seems a little confusing to me since I think of the "plugin" as the output .so file and it wouldn't be present beforehand, but that is just semantics. What about simplifying to "the plugin dependencies are present"? |
||
|
||
* The `execute` command does not require the plugin transformer dependencies be located in their `$GOPATH` directories, | ||
instead it expects a prebuilt .so file (of the name specified in the config file) to be in | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. really minor but "a prebuilt" seems unnecessary here |
||
`$GOPATH/src/github.com/vulcanize/vulcanizedb/plugins/` and, as noted above, also expects the plugin db migrations to | ||
have already been ran against the database. | ||
|
||
* Usage: | ||
* compose: `./vulcanizedb compose --config=./environments/config_name.toml` | ||
|
||
* execute: `./vulcanizedb execute --config=./environments/config_name.toml` | ||
|
||
* composeAndExecute: `./vulcanizedb composeAndExecute --config=./environments/config_name.toml` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need the |
||
|
||
### Flags | ||
The `compose` and `composeAndExecute` commands can be passed optional flags to specify the operation of the watchers: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
- `--recheck-headers`/`-r` - specifies whether to re-check headers for events after the header has already been queried for watched logs. | ||
|
@@ -50,7 +85,7 @@ Defaults to `false`. | |
Argument is expected to be a duration (integer measured in nanoseconds): e.g. `-q=10m30s` (for 10 minute, 30 second intervals). | ||
Defaults to `5m` (5 minutes). | ||
|
||
## Configuration | ||
### Configuration | ||
A .toml config file is specified when executing the commands. | ||
The config provides information for composing a set of transformers from external repositories: | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
# Repository Maintenance | ||
|
||
## Diagrams | ||
- Diagrams were created with [draw.io](draw.io). | ||
- To update a diagram: | ||
1. Go to [draw.io](draw.io). | ||
1. Click on *File > Open from* and choose the location of the diagram you want to update. | ||
1. Once open in draw.io, you may update it. | ||
1. Export the diagram to this repository's directory and add commit it. | ||
|
||
|
||
## Generating the Changelog | ||
We use [github-changelog-generator](https://github.com/github-changelog-generator/github-changelog-generator) to | ||
generate release Changelogs. To be consistent with previous Changelogs, the following flags should be passed to the | ||
command: | ||
|
||
``` | ||
--user vulcanize | ||
--project vulcanizedb | ||
--token {YOUR_GITHUB_TOKEN} | ||
--no-issues | ||
--usernames-as-github-logins | ||
--since-tag {PREVIOUS_RELEASE_TAG} | ||
``` | ||
|
||
More information on why your github token is needed, and how to generate it here:https://github | ||
.com/github-changelog-generator/github-changelog-generator#github-token |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gofmt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're specifically using the
go fmt
package, as opposed togofmt
- I had no idea they were two different things! 🤯There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! This is news to me also. Whoops, I probably should have noticed that in our Makefile now by now 🙃