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

Remove methods causing issues during initialization and creation of schema when database has different name than postgres #1842

Merged
merged 13 commits into from
Jun 23, 2020

Conversation

pacospace
Copy link
Contributor

@pacospace pacospace commented Jun 23, 2020

Related Issues and Dependencies

Due to issue in sqlalchmey_utils which use default postgres database name: kvesteri/sqlalchemy-utils#372

Fixes: thoth-station/thoth-application#41

This introduces a breaking change

  • Yes
  • No

This should yield a new module release

  • Yes
  • No

This Pull Request implements

Adapt methods that were causing issue when the name of the database is not postgres from sqlalchemy utils.

Francesco Murdaca added 6 commits June 22, 2020 17:21
Signed-off-by: Francesco Murdaca <fmurdaca@redhat.com>
Signed-off-by: Francesco Murdaca <fmurdaca@redhat.com>
Signed-off-by: Francesco Murdaca <fmurdaca@redhat.com>
Signed-off-by: Francesco Murdaca <fmurdaca@redhat.com>
Signed-off-by: Francesco Murdaca <fmurdaca@redhat.com>
Signed-off-by: Francesco Murdaca <fmurdaca@redhat.com>
@pacospace pacospace requested a review from fridex as a code owner June 23, 2020 08:19
@todo
Copy link

todo bot commented Jun 23, 2020

Remove once kvesteri/sqlalchemy-utils#372 is merged and new release of sqlalchemy_utils is released

#TODO: Remove once https://github.com/kvesteri/sqlalchemy-utils/pull/372 is merged and new release of sqlalchemy_utils is released
is_successfully_started = False
try:
self._engine = create_engine(self.construct_connection_string(), echo=echo)
self._sessionmaker = sessionmaker(bind=self._engine)
self._is_successfully_started = True


This comment was generated by todo based on a TODO comment in 3beaa1d in #1842. cc @pacospace.

@pacospace pacospace changed the title Add debug Remove methods causing issues during initialization and creation of schema when database has different name than postgres Jun 23, 2020
try:
self._engine = create_engine(self.construct_connection_string(), echo=echo)
self._sessionmaker = sessionmaker(bind=self._engine)
except Exception:
self._is_successfully_started = True
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a need for a new object attribute? If so it should go to the __init__ if we want to use it in other methods. Otherwise it can be a variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed and implemented methods in postgres_utils.py :)

@ghost
Copy link

ghost commented Jun 23, 2020

Build failed.

Signed-off-by: Francesco Murdaca <fmurdaca@redhat.com>
@todo
Copy link

todo bot commented Jun 23, 2020

Remove once kvesteri/sqlalchemy-utils#372 is merged and new release of sqlalchemy_utils is issued

#TODO: Remove once https://github.com/kvesteri/sqlalchemy-utils/pull/372 is merged and new release of sqlalchemy_utils is issued
is_successfully_started = False
try:
self._engine = create_engine(self.construct_connection_string(), echo=echo)
self._sessionmaker = sessionmaker(bind=self._engine)
is_successfully_started = True


This comment was generated by todo based on a TODO comment in 0e8223f in #1842. cc @pacospace.

@ghost
Copy link

ghost commented Jun 23, 2020

Build failed.

@todo
Copy link

todo bot commented Jun 23, 2020

remove once kvesteri/sqlalchemy-utils#372 is merged

# TODO: remove once https://github.com/kvesteri/sqlalchemy-utils/pull/372 is merged
#
###################################################################
# SQLAlchemy utils methods
# Copyright (c) 2012, Konsta Vesterinen
#


This comment was generated by todo based on a TODO comment in c5dae67 in #1842. cc @pacospace.

Signed-off-by: Francesco Murdaca <fmurdaca@redhat.com>
Signed-off-by: Francesco Murdaca <fmurdaca@redhat.com>
@ghost
Copy link

ghost commented Jun 23, 2020

Build failed.

Copy link
Contributor

@fridex fridex left a comment

Choose a reason for hiding this comment

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

let's have it in 👍

thanks! 👏

@sefkhet-abwy sefkhet-abwy bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 23, 2020
@fridex
Copy link
Contributor

fridex commented Jun 23, 2020

recheck

@ghost
Copy link

ghost commented Jun 23, 2020

Build failed.

@ghost
Copy link

ghost commented Jun 23, 2020

Build failed.

Signed-off-by: Francesco Murdaca <fmurdaca@redhat.com>
@ghost
Copy link

ghost commented Jun 23, 2020

Build failed.

@fridex
Copy link
Contributor

fridex commented Jun 23, 2020

recheck

@ghost
Copy link

ghost commented Jun 23, 2020

Build failed.

@ghost
Copy link

ghost commented Jun 23, 2020

Build failed.

Signed-off-by: Francesco Murdaca <fmurdaca@redhat.com>
@ghost
Copy link

ghost commented Jun 23, 2020

Build failed.

Signed-off-by: Francesco Murdaca <fmurdaca@redhat.com>
@ghost
Copy link

ghost commented Jun 23, 2020

Build failed.

@ghost
Copy link

ghost commented Jun 23, 2020

Build failed.

@fridex
Copy link
Contributor

fridex commented Jun 23, 2020

recheck

@ghost
Copy link

ghost commented Jun 23, 2020

Build succeeded.

@ghost
Copy link

ghost commented Jun 23, 2020

Build failed (gate pipeline). For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

@fridex
Copy link
Contributor

fridex commented Jun 23, 2020

I'll merge this manually to ship new release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

init-job default to postgres when trying to connect to database named thoth
2 participants