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

SFR-2245: Refactor dev setup process and fix infinite cluster loop #396

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

kylevillegas93
Copy link
Contributor

@kylevillegas93 kylevillegas93 commented Oct 10, 2024

Description

  • Refactors dev setup process
  • Removes the dev setup process docker container as it doesn't need to run every time you run docker compose up
  • Fixes infinite cluster loop bug where the cluster_status would always remain false if we failed to cluster

Testing

python main.py -p DevelopmentSetupProcess -e local

Comment on lines +81 to +110
def create_database(self, db_connection):
try:
db_connection.execute(
sa.text(f"CREATE DATABASE {os.environ['POSTGRES_NAME']}"),
)
except ProgrammingError:
pass
except Exception as e:
logger.exception('Failed to create database')
raise e

def create_database_user(self, db_connection):
try:
db_connection.execute(
sa.text(
f"CREATE USER {os.environ['POSTGRES_USER']} "
f"WITH PASSWORD '{os.environ['POSTGRES_PSWD']}'",
),
)
db_connection.execute(
sa.text(
f"GRANT ALL PRIVILEGES ON DATABASE {os.environ['POSTGRES_NAME']} "
f"TO {os.environ['POSTGRES_USER']}",
),
)
except ProgrammingError:
pass
except Exception as e:
logger.exception('Failed to create database user')
raise e
Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea to separate those lines of code into two separate methods for clarity.

Copy link
Contributor

@mitri-slory mitri-slory left a comment

Choose a reason for hiding this comment

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

These changes looks great.

@kylevillegas93 kylevillegas93 merged commit 8f54078 into main Oct 10, 2024
1 check passed
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