Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store Justifications along with the rest of the block #147

Open
insipx opened this issue Jan 8, 2021 · 5 comments · May be fixed by #309
Open

Store Justifications along with the rest of the block #147

insipx opened this issue Jan 8, 2021 · 5 comments · May be fixed by #309
Labels
difficulty-easy enhancement New feature or request good first issue Good for newcomers

Comments

@insipx
Copy link
Collaborator

insipx commented Jan 8, 2021

For one reason or another Justifications aren't stored in the Postgres Database.

Justification is present on the SignedBlock field of Block though, so it would be nice to store it if it exists. This requires a new SQL migration that creates a Justification column as a bytea field. It is possible for justification to be null for a block, so the columns should reflect this.

The SQL Inserts should be modified to add the justification as well:

impl<B> Insert for Block<B>

@insipx insipx added enhancement New feature or request good first issue Good for newcomers difficulty-easy labels Jan 8, 2021
@jakehemmerle
Copy link
Contributor

I'll work on this guy

@jakehemmerle
Copy link
Contributor

After trying for an hour or two to get the postgres scripts to work with a docker container, I created a docker-compose.yaml with the URL defaults that makes spinning it up the DB super easy for those (like me) who don't know postgres ins and outs. Is this something worth including in the PR?

version: '3'

services:
  # container name is:
  pg-substrate-archive:
    image: 'postgres:latest'
    ports:
      - 5432:5432 # localhost_port:container_port ; don't change container_port
    environment:
      POSTGRES_USER: postgres
      POSTGRES_PASSWORD: 123 # change me
      POSTGRES_DB: kusama # default db name

basically sets up a db instance with user/pass as postgres and 123, kusama as the database name, and binds container port 5432 to localhost:5432. Too easy.

docker tl;dr we can stick in the readme:
start it up : docker-compose up -d
shut it down: docker-compose down
delete it: docker container stop pg-substrate-archive && docker container rm pg-substrate-archive

@insipx
Copy link
Collaborator Author

insipx commented Jun 11, 2021

Yeah that would be great! Would start the journey to closing #206 as well, but yeah definitely a worthy addition imo

@dvdplm
Copy link
Contributor

dvdplm commented Jun 14, 2021

Is this something worth including in the PR?

I think it would be best as a separate PR; it's orthogonal to justifications.

I'm fine with anything that makes the first-use experience better but I do not want to bless Docker as the "preferred way" of running substrate-archive. :)

@jakehemmerle
Copy link
Contributor

jakehemmerle commented Jun 27, 2021

Since justifications are nullable values, I think this is blocked until this is solved: launchbadge/sqlx#1294 (comment).

@jakehemmerle jakehemmerle linked a pull request Jun 27, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty-easy enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants