Skip to content

1189 fix#52

Merged
elizabethengelman merged 2 commits intostagingfrom
1189-fix
Feb 21, 2020
Merged

1189 fix#52
elizabethengelman merged 2 commits intostagingfrom
1189-fix

Conversation

@elizabethengelman
Copy link
Copy Markdown

No description provided.

* [Guide](../../staging/libraries/shared/factories/storage/README.md)
* [Example](../../staging/libraries/shared/factories/storage/EXAMPLE.md)
* [Guide](../libraries/shared/factories/storage/README.md)
* [Example](../libraries/shared/factories/storage/EXAMPLE.md)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What's up with this diff? Were these broken previously, or does this change what branch we look at?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I noticed that these links weren't working - not sure entirely how staging in the link was affecting things, but I noticed when I removed it, the link would still redirect to staging. 🤔

Execute(diff types.PersistedDiff) error
KeccakContractAddress() common.Hash
GetStorageKeysLookup() interface{}
GetStorageKeysLookup() KeysLookup
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎉

if !ok {
return addressToKeys, fmt.Errorf("%v type incompatible. Should be a storage.KeysLookup", keysLookup)
}
keysLookup := transformer.GetStorageKeysLookup()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is it possible for this to return nil? I guess that would probably be in a catastrophic config situation, but maybe worth still tossing up an error if that happens?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yep, great idea.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Looking at it a bit closer, we'd actually panic if the StorageKeysLookup wasn't set on the Transformer before making it here because when creating a new transformer we call SetDB on the StorageKeysLookup in NewTransformer. 🤔

Not sure I want to make this change now, but I wonder if we should consider returning an error from NewTransformer if either StorageKeysLookup or Repository are nil?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sounds good 👍

@elizabethengelman elizabethengelman merged commit 24d84f7 into staging Feb 21, 2020
@elizabethengelman elizabethengelman deleted the 1189-fix branch February 21, 2020 21:27
elizabethengelman pushed a commit that referenced this pull request Feb 25, 2020
* (VDB-1164) Get node name from web3_clientVersion

Use IPC path as node ID

* 1189 new move transformer interfaces (#48)

* Move storage Transformer interface to storage pkg

* Handle new storage transformer interface in plugin writer

* Add additional storage value loader logging

* Move event transformer interface to factories/event pkg

* Handle new event transformer interface location in plugin writer

* Memoize an transformer's keccakedAddress

* Update resetHeaderCheckCount docker setup (#50)

* 1189 fix (#52)

* Update ITransformer.GetStorageKeysLookup to return a KeysLookup

* Update custom-transformers.md with new transformer paths

* TO-879:docker ci (#40)

* dockerhub via travis

* add extract-diffs

* add reset-header-check

* (VDB-1207) Remove non-essential smart comments (#53)

- configure via postgraphile.tags.json5 instead

* Update postgres version in travis build (#54)

* Update postgres version in travis build to 11.6

this is the most recent posgres version available in RDS at the moment

* update travis secrets (#55)

Co-authored-by: Gabe Laughlin <gabe@bistrotech.net>
Co-authored-by: daimesava <56128157+daimesava@users.noreply.github.com>
Co-authored-by: Rob Mulholand <rmulholand@8thlight.com>
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