Skip to content

VDB-1368: Add Dockerfile for extract diffs cmd#259

Merged
elizabethengelman merged 5 commits intostagingfrom
vdb-1368-add-extract-diffs-dockerfile
Aug 11, 2020
Merged

VDB-1368: Add Dockerfile for extract diffs cmd#259
elizabethengelman merged 5 commits intostagingfrom
vdb-1368-add-extract-diffs-dockerfile

Conversation

@elizabethengelman
Copy link
Copy Markdown
Contributor

@elizabethengelman elizabethengelman commented Aug 3, 2020

This PR needs to work with this VDB PR. Before the VDB PR is merged in, pass VDB_VERSION=VDB-1368-add-new-geth-patch-as-diff-source as a build arg while building this container, eg:

docker build -f dockerfiles/extract_diffs/Dockerfile . -t extract --build-arg VDB_VERSION=VDB-1368-add-new-geth-patch-as-diff-source

@elizabethengelman elizabethengelman force-pushed the vdb-1368-add-extract-diffs-dockerfile branch from d1e72e7 to ae5fcdb Compare August 3, 2020 15:27
@elizabethengelman elizabethengelman marked this pull request as draft August 3, 2020 15:27
@elizabethengelman elizabethengelman changed the title VDB-1388: Add Dockerfile for extract diffs cmd VDB-1368: Add Dockerfile for extract diffs cmd Aug 3, 2020

# Run extractDiffs
echo "Running extractDiffs..."
./vulcanizedb extractDiffs --watchedAddresses=0x78f2c2af65126834c51822f56be0d7469d7a523e \
Copy link
Copy Markdown
Contributor Author

@elizabethengelman elizabethengelman Aug 3, 2020

Choose a reason for hiding this comment

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

Unfortunately, I don't think that we can add a comment for each address telling which contract it belongs to. Not sure if that's ideal, so I may play around with just using a .toml config file like we're doing with execute.

@elizabethengelman elizabethengelman marked this pull request as ready for review August 3, 2020 22:34
Copy link
Copy Markdown
Contributor

@yaoandrew yaoandrew left a comment

Choose a reason for hiding this comment

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

LGTM - I was able to build the container locally. Of course I didn't read your note at first so I got an error when I tried to run it without --build-arg set🤣

@elizabethengelman elizabethengelman force-pushed the vdb-1368-add-extract-diffs-dockerfile branch 2 times, most recently from 980cee3 to 7223a43 Compare August 7, 2020 17:00
@elizabethengelman
Copy link
Copy Markdown
Contributor Author

I've added extract diffs to the travis deploy script since the last review and would love another set of eyes!

Copy link
Copy Markdown
Contributor

@rmulhol rmulhol left a comment

Choose a reason for hiding this comment

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

A couple questions, but LGTM 👍

address = "0x956ecD6a9A9A0d84e8eB4e6BaaC09329E202E55e"
abi = '[{"anonymous":false,"inputs":[{"indexed":false,"internalType":"uint256","name":"val","type":"uint256"},{"indexed":false,"internalType":"uint256","name":"age","type":"uint256"}],"name":"LogMedianPrice","type":"event"},{"anonymous":true,"inputs":[{"indexed":true,"internalType":"bytes4","name":"sig","type":"bytes4"},{"indexed":true,"internalType":"address","name":"usr","type":"address"},{"indexed":true,"internalType":"bytes32","name":"arg1","type":"bytes32"},{"indexed":true,"internalType":"bytes32","name":"arg2","type":"bytes32"},{"indexed":false,"internalType":"bytes","name":"data","type":"bytes"}],"name":"LogNote","type":"event"},{"constant":true,"inputs":[],"name":"age","outputs":[{"internalType":"uint32","name":"","type":"uint32"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[],"name":"bar","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[{"internalType":"address","name":"","type":"address"}],"name":"bud","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"internalType":"address","name":"usr","type":"address"}],"name":"deny","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"internalType":"address[]","name":"a","type":"address[]"}],"name":"diss","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"internalType":"address","name":"a","type":"address"}],"name":"diss","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"internalType":"address[]","name":"a","type":"address[]"}],"name":"drop","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"internalType":"address[]","name":"a","type":"address[]"}],"name":"kiss","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"internalType":"address","name":"a","type":"address"}],"name":"kiss","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"internalType":"address[]","name":"a","type":"address[]"}],"name":"lift","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":true,"inputs":[{"internalType":"address","name":"","type":"address"}],"name":"orcl","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[],"name":"peek","outputs":[{"internalType":"uint256","name":"","type":"uint256"},{"internalType":"bool","name":"","type":"bool"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"internalType":"uint256[]","name":"val_","type":"uint256[]"},{"internalType":"uint256[]","name":"age_","type":"uint256[]"},{"internalType":"uint8[]","name":"v","type":"uint8[]"},{"internalType":"bytes32[]","name":"r","type":"bytes32[]"},{"internalType":"bytes32[]","name":"s","type":"bytes32[]"}],"name":"poke","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":true,"inputs":[],"name":"read","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"internalType":"address","name":"usr","type":"address"}],"name":"rely","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"internalType":"uint256","name":"bar_","type":"uint256"}],"name":"setBar","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":true,"inputs":[{"internalType":"uint8","name":"","type":"uint8"}],"name":"slot","outputs":[{"internalType":"address","name":"","type":"address"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[{"internalType":"address","name":"","type":"address"}],"name":"wards","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[],"name":"wat","outputs":[{"internalType":"bytes32","name":"","type":"bytes32"}],"payable":false,"stateMutability":"view","type":"function"}]'
deployed = 10322821
[contract.OASIS_MATCHING_MARKET_ONE]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Might be able to remove the Oasis + OSM contracts?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in c0947f8

HEALTHCHECK CMD test -f /tmp/connection

# need to execute with a shell to access env variables
CMD ["./startup_script.sh"] No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One thing that bit me recently was forgetting to update permissions on the startup_script.sh file - if you haven't run this locally, maybe worth verifying that won't be an issue here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this should be okay - I've changed the script's permissions, and can run the docker container on my machine.

docker build -f dockerfiles/execute/Dockerfile . -t makerdao/vdb-execute:$TAG

message BUILDING EXTRACT-DIFFS DOCKER IMAGE
docker build -f dockerfiles/extract_diffs/Dockerfile . -t makerdao/vdb-extract-diffs:$TAG
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is moving over here, do we want a corresponding PR to remove it from vdb?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👍 I think that sky-ecosystem/vulcanizedb#119 should take care of this.

@elizabethengelman elizabethengelman force-pushed the vdb-1368-add-extract-diffs-dockerfile branch from c0947f8 to 8f3e735 Compare August 11, 2020 14:48
@elizabethengelman elizabethengelman merged commit 182d8be into staging Aug 11, 2020
@elizabethengelman elizabethengelman deleted the vdb-1368-add-extract-diffs-dockerfile branch August 11, 2020 18:35
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