Skip to content

Updating README to explain current usage#87

Merged
elizabethengelman merged 2 commits intostagingfrom
vdb-1010-update-readme
Jan 16, 2020
Merged

Updating README to explain current usage#87
elizabethengelman merged 2 commits intostagingfrom
vdb-1010-update-readme

Conversation

@elizabethengelman
Copy link
Copy Markdown
Contributor

A couple of things that I think would be helpful to add:

  • how to add a new transformer
  • how to use/query the graphiql API

@elizabethengelman elizabethengelman force-pushed the vdb-1010-update-readme branch 3 times, most recently from 685432e to b03470d Compare January 9, 2020 14:26
@elizabethengelman elizabethengelman changed the title Updating README to be explain current usage Updating README to explain current usage Jan 9, 2020
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.

Thanks for taking the lead on this!


### Getting the project
Download the transformer codebase to your local local `GOPATH` via:
`go get github.com/makerdao/vdb-mcd-transformers`
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.

Does this work? I thought we might not be able to use go get as long as we're importing the Geth fork

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.

Good call, I had forgotten about that. I think go get should work for getting the transformers repo - since it doesn't directly depend on geth, but geth through VDB. But you're totally right that we'll probably have to use git clone for the vdb repo.

That said, I wonder if we should use the same method for both repos, just for clarity/consistency?

README.md Outdated
These core commands can be run via Docker images which is the preferred method, or they can be built and run via the
command line interface. In either method, a postgres database will first need to be created:
1. Install Postgres
1. Create a superuser for yourself and make sure `psql --list` works without prompting for a password.
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.

Do you know if this is still necessary? Thinking we might be able to avoid it as long as the db user/pw are included in config

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.

It looks like we still need to create a supersuser for creating the citext extension which we're using to allow for case insensitivity for urn identifiers.

CREATE EXTENSION IF NOT EXISTS citext;
ERROR:  permission denied to create extension "citext"
HINT:  Must be superuser to create this extension.

I wonder if we should invest some time into figuring out a way around this, so we can remove the necessity for superusers.

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.

I might be focused too much on the part that says "make sure psql --list works without prompting for a password" - thinking maybe we can avoid that as long as the user/pw has the right permissions to run the migrations

Copy link
Copy Markdown
Contributor Author

@elizabethengelman elizabethengelman Jan 13, 2020

Choose a reason for hiding this comment

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

gotcha, that makes sense to me. does the change in f69b277 work?

@elizabethengelman elizabethengelman merged commit e750595 into staging Jan 16, 2020
@elizabethengelman elizabethengelman deleted the vdb-1010-update-readme branch January 16, 2020 19:21
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