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

Adding lookup plugin for postgresql database #11315

Open
wants to merge 1 commit into
base: devel
from

Conversation

@gshivani

gshivani commented Jun 18, 2015

SUMMARY

lookup plugin for querying postgresql

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

lib/ansible/plugins/lookup/postgresql.py

ANSIBLE VERSION

2.3

ADDITIONAL INFORMATION
@msabramo

This comment has been minimized.

Contributor

msabramo commented Jun 28, 2015

Maybe even more useful would be a more general plugin that uses SQLAlchemy to query the database. Then you could use it with any of the database engines supported by SQLAlchemy.

http://docs.sqlalchemy.org/en/rel_1_0/dialects/index.html

Instead of taking username, password, etc., it could take a SQLAlchemy database URL (which probably a lot of people would keep in variables and encrypt with vault).

@amenonsen

This comment has been minimized.

Contributor

amenonsen commented Jul 29, 2015

A few quick thoughts: (a) this definitely needs documentation, (b) "connection failed or something's wrong" is a complete non-starter, this is something that may well not work right the first time, and that error would drive me nuts while debugging and not help at all.

@msabramo's suggestion to use SQLAlchemy is… well, it's probably the Right Thing To Do™, though I personally don't want to have to install and learn SQLAlchemy and don't need anything but Postgres support. Being able to encrypt and store database connection information is certainly nice, but it's also possible to encrypt and store the conninfo string for Postgres with this PR already.

@ansibot

This comment has been minimized.

Contributor

ansibot commented Apr 4, 2017

@gshivani Greetings! Thanks for taking the time to open this pullrequest. In order for the community to handle your pullrequest effectively, we need a bit more information.

Here are the items we could not find in your description:

  • issue type
  • ansible version
  • component name

Please set the description of this pullrequest with this template:
https://raw.githubusercontent.com/ansible/ansible/devel/.github/PULL_REQUEST_TEMPLATE.md

click here for bot help

@ansibot ansibot added the stale_ci label Apr 12, 2017

@bcoca bcoca self-assigned this Jul 17, 2017

@bcoca

I know this has been open a long time, but since previous comments were not addressed we have not returned to it. Now we are trying to close these old issues and if we don't get any response to this review either we'll end up just closing the PR.

from __future__ import (absolute_import, division, print_function)
__metaclass__ = type
import psycopg2

This comment has been minimized.

@bcoca

bcoca Jul 17, 2017

Member

this needs to be guarded and return an ansible error in the init (see consul_kv lookup and HAS_CONSUL)

row_string = str(row)
ret.append(row_string)
except:
raise AnsibleError("Connection Failed or something's wrong")

This comment has been minimized.

@bcoca

bcoca Jul 17, 2017

Member

Adding the exception string to the error would help the user narrow down what the issue is.

user = paramvals['user']
password = paramvals['password']
sql = paramvals['sql']
#Connection to postgresql

This comment has been minimized.

@bcoca

bcoca Jul 17, 2017

Member

lacking parameter validation, user should be notified if the minimal requirements for lookup to work are not met

@ansibot ansibot removed the new_contributor label Nov 3, 2017

@ansibot ansibot added feature and removed feature_pull_request labels Mar 2, 2018

@bcoca bcoca closed this Apr 20, 2018

@bcoca bcoca reopened this Apr 20, 2018

@ansibot ansibot removed the stale_ci label Apr 20, 2018

@ansibot ansibot added the stale_ci label Apr 28, 2018

@ansibot ansibot added the new_plugin label May 23, 2018

@ansibot ansibot removed the stale_ci label May 31, 2018

@jstoja

This comment has been minimized.

jstoja commented Sep 13, 2018

@bcoca No answer after +1y, you can probably close it.

@ryanmerolle

This comment has been minimized.

Contributor

ryanmerolle commented Oct 9, 2018

What is required here to make this shippable? I am interested in this becoming a standard lookup plugin in ansible.

'user' : 'user',
'password' : 'password',
'sql' : 'query',
}

This comment has been minimized.

@ryanmerolle

ryanmerolle Oct 9, 2018

Contributor

It likely makes sense to add the remaining psycopg2 parameters including host and port.

@gundalow

This comment has been minimized.

Contributor

gundalow commented Oct 10, 2018

@ryanmerolle PR needs rebasing https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html

It maybe worth you creating a new branch and copying across the new file into that branch

@gshivani

This comment has been minimized.

gshivani commented Oct 11, 2018

@ryanmerolle Glad that you are interested in this becoming a standard plugin! I'm not sure if I can get to this soon. Please feel free to create new branch and reuse the code!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment