Skip to content

Conversation

@julienloizelet
Copy link
Collaborator

Fixes #19

@julienloizelet julienloizelet requested a review from rr404 February 15, 2024 04:55
@julienloizelet julienloizelet self-assigned this Feb 15, 2024
engine = create_engine(connection_string, echo=False)
Base.metadata.create_all(engine)
Session = sessionmaker(bind=engine)
self.session = Session()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we believe chatGPT:

The Session() call creates a new SQLAlchemy Session, which opens a new transaction. It's not recommended to start a transaction in the init method.

The reason is that the init method is called when an instance of the class is created. If you start a transaction in the init method, the transaction will be open for the entire lifetime of the object, which is not what you typically want. Transactions should be short-lived and should be started just before performing database operations, and ended (committed or rolled back) as soon as possible.

Instead, you should create the sessionmaker in the init method and then start a new session in each method that performs database operations, using a context manager (with statement). This ensures that each session is properly closed, even if an error occurs during the database operations.

Copy link
Collaborator

@rr404 rr404 left a comment

Choose a reason for hiding this comment

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

LGTM

@julienloizelet julienloizelet merged commit b2ce126 into crowdsecurity:main Feb 16, 2024
julienloizelet added a commit to julienloizelet/crowdsec-python-capi-sdk that referenced this pull request Mar 14, 2024
)

* feat(sql session): Use context manager for sql session

* docs(changelog): Prepare next release
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.

Better handling of sql session

2 participants