Skip to content

Update error message for watch#485

Closed
djake wants to merge 1 commit intographile:masterfrom
djake:master
Closed

Update error message for watch#485
djake wants to merge 1 commit intographile:masterfrom
djake:master

Conversation

@djake
Copy link

@djake djake commented Jul 4, 2019

Hello, this is a first PR for this project. I hope you don't mind I skipped the "ask first" instruction because this is a minor change to an error message, so simply typing this description was more time consuming than the actual PR.

While trying to setup new roles, I broke watching and got an error like:

Failed to setup watch fixtures in Postgres database ️️⚠️
This is likely because the PostgreSQL user in the connection string does not have sufficient privileges; you can solve this by passing the 'owner' connection string via '--owner-connection' / 'ownerConnectionString'. If the fixtures already exist, the watch functionality may still work.
Enable DEBUG='graphile-build-pg' to see the error

I am using the CLI and passing configuration via the options prop in .postgraphilerc.js. Per this message, I added options.ownerConnectionString. It appears that it should be options.ownerConnection. My guess is the existing language is meant for library.ownerConnection. Anyhow, I thought this PR might provide some clarity for any future newbie who stumbles on it.

If this is unwelcome or simply reflects some user error on my part, feel free to close. If you think it might be preferable to use the same property name in both cases or some other alternative, I'm happy to try that instead, I would like to contribute in some way as I make use of this project.

@benjie
Copy link
Member

benjie commented Jul 5, 2019

Hi @djake; thanks for your PR! The "ask first" is just there to help avoid people getting frustrated if their hard work were to be rejected.

You're absolutely right that the CLI option is ownerConnection; everything in .postgraphilerc.js is a camelCase'd version of the CLI arguments (in this case --owner-connection) rather than library options.

I'm very unhappy with this state of affairs; so am planning to replace it:

graphile/crystal#705

As such, I'd rather not merge this PR because I'd rather it was not necessary at all. I'm going to leave it open to nag me to address the underlying problem of postgraphilerc.

Thanks again!

@benjie
Copy link
Member

benjie commented Aug 5, 2019

On second thoughts I'm closing it. I know the RC needs dealing with 👍

@benjie benjie closed this Aug 5, 2019
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