Skip to content

Automatic addition of database before connecting to it #4316

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

Merged
merged 22 commits into from
Nov 29, 2024

Conversation

feyruzb
Copy link
Collaborator

@feyruzb feyruzb commented Aug 16, 2024

Introduced a new function add_product_support() using SQLAlchemy to enhance the addProduct() function.
Automates the creation of the database for PostgreSQL and SQLite, eliminating the need for a manual script before connection.
Note: SQLite already supports automatic database creation.
Note2: The deletion of product doesn't delete the Postgres database, it was intentional , in case of accidental removal

@feyruzb feyruzb force-pushed the improvement_of_new_product branch 6 times, most recently from 01343fb to e39cd16 Compare August 16, 2024 16:38
@feyruzb feyruzb force-pushed the improvement_of_new_product branch from e39cd16 to 32aae73 Compare August 16, 2024 16:46
@feyruzb feyruzb force-pushed the improvement_of_new_product branch 12 times, most recently from 0f4c534 to 8d84c11 Compare August 22, 2024 13:28
@feyruzb feyruzb force-pushed the improvement_of_new_product branch 5 times, most recently from e20770d to 451b2a2 Compare August 23, 2024 16:50
@feyruzb feyruzb force-pushed the improvement_of_new_product branch from f9dffb1 to 2bed7bb Compare September 2, 2024 15:11
@feyruzb feyruzb force-pushed the improvement_of_new_product branch from 1111841 to e25167e Compare September 2, 2024 16:37
@feyruzb feyruzb force-pushed the improvement_of_new_product branch 3 times, most recently from 3beb9b8 to 59dcff3 Compare September 3, 2024 10:03
@feyruzb feyruzb force-pushed the improvement_of_new_product branch from 59dcff3 to b95b3aa Compare September 3, 2024 11:58
@feyruzb feyruzb force-pushed the improvement_of_new_product branch from 82205e7 to 078f1a3 Compare September 3, 2024 12:55
@feyruzb feyruzb self-assigned this Oct 16, 2024
@dkrupp dkrupp added this to the release 6.25.0 milestone Nov 5, 2024
@dkrupp dkrupp requested a review from cservakt November 5, 2024 14:23
@@ -306,6 +309,58 @@ def getProductConfiguration(self, product_id):

return prod

@timeit
def add_product_support(self, product):
Copy link
Contributor

Choose a reason for hiding this comment

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

Functions that are not on the API could be "private", i.e. having double underscore prefix in their name: __add_product_support.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

to assist addProduct() function that connects to
an already existing database.
"""
self.__require_permission([permissions.SUPERUSER])
Copy link
Contributor

Choose a reason for hiding this comment

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

Only API functions need to check permission. This function can't be called from outside.

Copy link
Collaborator Author

@feyruzb feyruzb Nov 25, 2024

Choose a reason for hiding this comment

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

fixed, check removed

@@ -912,6 +915,46 @@ def add_product(self, orm_product, init_db=False):

self.__products[prod.endpoint] = prod

def get_if_database_in_use(self, product):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def get_if_database_in_use(self, product):
def is_database_used(self, connection):

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

"""
# get the database name from the product connection string
is_sqlite = False
if product.database.endswith('.sqlite'):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if product.database.endswith('.sqlite'):
if product.engine == 'sqlite':

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed


LOG.info("The database string that will be added: %s", to_add)

LOG.info("to add: %s", to_add)
Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary log.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

]

# True if found, False otherwise
LOG.info("dynamic list: %s", dynamic_list)
Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary log.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

# remove the first 16 charecters because in query it is
# included in the path string
dynamic_list = [
d[16:] if d.endswith('.sqlite') else d
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't rely on the length of a specific connection string prefix. d.split(':', 1) would split the connection string on the first : character.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed


# remove the first 16 charecters because in query it is
# included in the path string
dynamic_list = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be merged with the previous list comprehension in line 936.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed


# get the current state of connected databases from products
dynamic_list = [
a.connection.replace('////', '/')
Copy link
Contributor

Choose a reason for hiding this comment

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

SQLAlchemy can be used for parsing the connection string: https://docs.sqlalchemy.org/en/20/core/engines.html#sqlalchemy.engine.make_url
This could be used for a better check condition in the loop at line 951.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@feyruzb feyruzb force-pushed the improvement_of_new_product branch from 054fa4b to 14414fe Compare November 26, 2024 15:08
@feyruzb feyruzb force-pushed the improvement_of_new_product branch from 3748e36 to 09f1ae7 Compare November 28, 2024 14:20
@feyruzb feyruzb force-pushed the improvement_of_new_product branch from 09f1ae7 to 83177b0 Compare November 28, 2024 14:29
@bruntib bruntib merged commit 7aa78ab into Ericsson:master Nov 29, 2024
7 of 8 checks passed
@feyruzb feyruzb deleted the improvement_of_new_product branch May 5, 2025 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants