https://homeproj.cesnet.cz/https://homeproj.cesnet.cz/httpauth-login/favicon.ico?16194486082020-01-24T14:32:51ZHomeproj: Redmine for CESNETMentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=261662020-01-24T14:32:51ZJan Machjan.mach@cesnet.cz
<ul><li><strong>Related to</strong> <i><a class="issue tracker-1 status-5 priority-3 priority-lowest closed" href="/issues/4251">Bug #4251</a>: User data access through SQLAlchemy keeps a transaction open</i> added</li></ul> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=266382020-04-07T07:50:51ZJan Machjan.mach@cesnet.cz
<ul><li><strong>Target version</strong> changed from <i>2.7</i> to <i>2.8</i></li></ul> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=291002020-11-05T10:34:31ZRajmund Hruška
<ul><li><strong>Assignee</strong> changed from <i>Jan Mach</i> to <i>Rajmund Hruška</i></li></ul> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=304732020-12-21T08:38:14ZRajmund Hruška
<ul><li><strong>To be discussed</strong> changed from <i>No</i> to <i>Yes</i></li></ul> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=304752020-12-21T15:24:21ZRadko Krkoškrkos@cesnet.cz
<ul></ul><p>This issue stems from <a class="issue tracker-1 status-5 priority-3 priority-lowest closed" title="Bug: User data access through SQLAlchemy keeps a transaction open (Closed)" href="https://homeproj.cesnet.cz/issues/4251">#4251</a>, where a workaround for a specific transaction handling bug was found and recommended. Based on the videocall discussion a more streamlined approach is required, possibly by managing the transaction lifetime by limiting the life of a <code>Session</code> object.<br />Following the official documentation [1], the upcoming version of <code>SQLAlchemy</code> (v1.4), currently in beta, includes a <code>Python</code>'s context manager based workflow with <code>Session</code> as a transaction manager and <code>Engine</code> as some form of a connection manager.<br />Based on [1], the best approach would be to keep a reference to <code>Engine</code>, instantiate a <code>Session</code> from it using a context manager and perform the data-focused actions inside that context. This is perhaps better documented by a code excerpt from [1]:</p>
<pre>
# create session and add objects
with Session(engine) as session, session.begin():
session.add(some_object)
session.add(some_other_object)
# inner context calls session.commit(), if there were no exceptions
# outer context calls session.close()
</pre>
<p>The current version 1.3 does not support the context manager approach while supporting the <code>Engine</code>/<code>Session</code> distinction, so a similar solution is possible [2], but the user (programmer) must both keep a reference to <code>sessionmaker</code> and carefully manage the lifetime of the <code>Session</code> object, what is error prone. It should nevertheless be a feasible approach.</p>
<p>Waiting for the next version might make sense here, although it is of note that the minimum required version of <code>Python</code> is bumped to 3.6 there [3] (not an issue for us as we are currently using 3.7.3, but might cause problems elsewhere). The migration document also promises various performance improvements in the next version.</p>
<p>The required changes to the code are non-trivial for both versions and the migration to v1.4 also seems to require some rewrites. I advise to do both at the same time and postpone this issue until after the <code>SQLAlchemy v1.4</code> release.</p>
<p>[1] <a class="external" href="https://docs.sqlalchemy.org/en/14/orm/session_basics.html">https://docs.sqlalchemy.org/en/14/orm/session_basics.html</a><br />[2] <a class="external" href="https://docs.sqlalchemy.org/en/13/orm/session_basics.html">https://docs.sqlalchemy.org/en/13/orm/session_basics.html</a><br />[3] <a class="external" href="https://docs.sqlalchemy.org/en/14/changelog/migration_14.html">https://docs.sqlalchemy.org/en/14/changelog/migration_14.html</a></p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=304762020-12-22T06:57:22ZRadko Krkoškrkos@cesnet.cz
<ul></ul><p>As for the <code>flask-sqlalchemy</code>, it is just a simple adaptor [1] and seems to take the <code>sessionmaker</code> route [2]. It also seems to manage the lifetime of the session subobject. Although it is not obvious from the example code [3] (why would an object's attribute lifetime be different from the object itself?), the documentation refers to the <code>session</code> as scoped and declares [4]:</p>
<blockquote>
<p>You have to commit the session, but you don’t have to remove it at the end of the request, Flask-SQLAlchemy does that for you.</p>
</blockquote>
<p>So the way of <code>flask-sqlalchemy</code> should be a viable approach to correctly access the database and if not, it is a bug somewhere in the <code>flask-sqlalchemy</code>\<code>SQLAlchemy</code>\<code>psycopg2</code> stack and should be treated as such.</p>
<p>This is all based on code review and study of the documentation, not tested hard evidence.</p>
<p>[1] <a class="external" href="https://github.com/pallets/flask-sqlalchemy/blob/master/src/flask_sqlalchemy/model.py">https://github.com/pallets/flask-sqlalchemy/blob/master/src/flask_sqlalchemy/model.py</a><br />[2] <a class="external" href="https://flask-sqlalchemy.palletsprojects.com/en/2.x/api/#flask_sqlalchemy.SQLAlchemy">https://flask-sqlalchemy.palletsprojects.com/en/2.x/api/#flask_sqlalchemy.SQLAlchemy</a><br />[3] <a class="external" href="https://flask-sqlalchemy.palletsprojects.com/en/2.x/quickstart/#a-minimal-application">https://flask-sqlalchemy.palletsprojects.com/en/2.x/quickstart/#a-minimal-application</a><br />[4] <a class="external" href="https://flask-sqlalchemy.palletsprojects.com/en/2.x/quickstart/#road-to-enlightenment">https://flask-sqlalchemy.palletsprojects.com/en/2.x/quickstart/#road-to-enlightenment</a></p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=304792021-01-04T11:51:41ZRadko Krkoškrkos@cesnet.cz
<ul><li><strong>File</strong> <a href="/attachments/3592">test_flask_sqlalchemy.py</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/3592/test_flask_sqlalchemy.py">test_flask_sqlalchemy.py</a> added</li></ul><p>Further investigating the <code>flask-sqlalchemy</code>, a simple testing script was prepared (attached), to determine the session management behaviour. The output, especially section "Commit - transaction start", shows that <code>commit()</code> in fact does not immediately start another transaction and that is deferred until the next query. Therefore, a viable approach using <code>flask-sqlalchemy</code> is:</p>
<pre>
from flask import Flask
from flask_sqlalchemy import SQLAlchemy
app = Flask(__name__)
db = SQLAlchemy(app)
# New transaction
db.session.query(...)
db.session.commit()
# New transaction
db.session.query(...)
db.session.commit()
...
</pre>
<p>as opposed to <code>rollback()</code> that is required using the current architecture based on pure <code>SQLAlchemy</code>.</p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=308632021-01-13T13:25:49ZPavel Káchaph@cesnet.cz
<ul></ul><p>Consensus on 2021-01-11 meeting was that the correct way most probably is switching to flask-sqlalchemy, which hides some sqlalchemy idiosyncracies. We would than be able to remove weird rollback introduced in <a class="issue tracker-1 status-5 priority-3 priority-lowest closed" title="Bug: User data access through SQLAlchemy keeps a transaction open (Closed)" href="https://homeproj.cesnet.cz/issues/4251">#4251</a>.</p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=310392021-01-22T08:56:27ZPavel Káchaph@cesnet.cz
<ul><li><strong>To be discussed</strong> deleted (<del><i>Yes</i></del>)</li></ul> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=320282021-02-19T09:57:54ZPavel Káchaph@cesnet.cz
<ul><li><strong>Target version</strong> changed from <i>2.8</i> to <i>2.9</i></li></ul> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=357542021-10-07T13:03:43ZRajmund Hruška
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Feedback</i></li><li><strong>To be discussed</strong> set to <i>Yes</i></li></ul><p>I tried using flask_sqlalchemy to get rid of that rollback in <a class="changeset" title="Feature: Use SQLAlchemy from flask_sqlalchemy. (Redmine issue: #6205)" href="https://homeproj.cesnet.cz/projects/mentat/repository/mentat-ng/revisions/0d12bd68d3aff5538908bad8c64cb3205f18c1f9">0d12bd68</a>. Although the code is executable and all tests passed, when I check the running queries I see that the query from <a class="issue tracker-1 status-5 priority-3 priority-lowest closed" title="Bug: User data access through SQLAlchemy keeps a transaction open (Closed)" href="https://homeproj.cesnet.cz/issues/4251">#4251</a> is still running. So the original problem is not solved yet.</p>
<p>From my understanding, I used the flask_sqlalchemy in the same way as in the example posted in <a href="#note-7">#note-7</a> so it should be working. That is not the case though.</p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=357552021-10-08T06:49:35ZRadko Krkoškrkos@cesnet.cz
<ul></ul><p>Rajmund Hruska wrote in <a href="#note-11">#note-11</a>:</p>
<blockquote>
<p>From my understanding, I used the flask_sqlalchemy in the same way as in the example posted in <a href="#note-7">#note-7</a> so it should be working.</p>
</blockquote>
<p>Not quite. You need to signal an end of transaction to the ORM somehow. In <a href="#note-7">#note-7</a>, it is done by calling <code>db.session.commit()</code>. In <code>whois.py</code>, after being done with the loop of queries (somewhere about where previously the <code>storage.session.rollback()</code> was), the <code>storage.session.commit()</code> method should be called. That should close the running transaction, without it, the transaction will remain open. The symptoms are the same as for <a class="issue tracker-1 status-5 priority-3 priority-lowest closed" title="Bug: User data access through SQLAlchemy keeps a transaction open (Closed)" href="https://homeproj.cesnet.cz/issues/4251">#4251</a>, but the cause is not a deficiency in the new ORM (as was with the prior one), but in the way it is used, see <a href="#note-6">#note-6</a> and original the source [4,<a href="#note-6">#note-6</a>] for more details.</p>
<p>I did not run any code now, so take all this with a grain of salt. Is my reasoning understandable?</p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=357582021-10-08T08:36:12ZRadko Krkoškrkos@cesnet.cz
<ul></ul><p>As was discussed on the meeting, with regards also to <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Hawat DB connection is not recreated if dropped (Closed)" href="https://homeproj.cesnet.cz/issues/4569">#4569</a>, access to SQLAlchemy's internals is needed even in the <code>flask_sqlalchemy</code>. Based on the documentation [1], this might work:</p>
<pre>
from flask_sqlalchemy import BaseQuery
</pre>
<p>[1] <a class="external" href="https://flask-sqlalchemy.palletsprojects.com/en/2.x/customizing/#query-class">https://flask-sqlalchemy.palletsprojects.com/en/2.x/customizing/#query-class</a></p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=357592021-10-08T09:03:06ZRajmund Hruška
<ul><li><strong>Status</strong> changed from <i>Feedback</i> to <i>In Progress</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>50</i></li><li><strong>To be discussed</strong> changed from <i>Yes</i> to <i>No</i></li></ul><p>Radko Krkoš wrote in <a href="#note-13">#note-13</a>:</p>
<blockquote>
<p>As was discussed on the meeting, with regards also to <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Hawat DB connection is not recreated if dropped (Closed)" href="https://homeproj.cesnet.cz/issues/4569">#4569</a>, access to SQLAlchemy's internals is needed even in the <code>flask_sqlalchemy</code>. Based on the documentation [1], this might work:</p>
<p>[...]</p>
<p>[1] <a class="external" href="https://flask-sqlalchemy.palletsprojects.com/en/2.x/customizing/#query-class">https://flask-sqlalchemy.palletsprojects.com/en/2.x/customizing/#query-class</a></p>
</blockquote>
<p>I found out that <code>Query</code> can be found at <code>flask_sqlalchemy.orm.Query</code>, so I used that class.</p>
<p>I can't fully remove <code>sqlalchemy</code> module because <code>sqlalchemy.exc.OperationalError</code> is used and <code>exc</code> module is <a href="https://github.com/pallets/flask-sqlalchemy/issues/764" class="external">not present in the <code>flask_sqlalchemy</code> module</a>.</p>
<p>In <a class="changeset" title="Fix: Use RetryingQuery again. (Redmine issue: #6205)" href="https://homeproj.cesnet.cz/projects/mentat/repository/mentat-ng/revisions/7c1dce70526bfc93f97b5175e7ade2356266b4d3">7c1dce70</a> I also added the <code>RetryingQuery</code> back, which I removed in the previous commit just for test purposes.</p>
<p>As it was discussed during the meeting, I will check if <code>flask_sqlalchemy</code> can be used at other places where insert/delete queries are used.</p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=357602021-10-08T09:35:48ZRadko Krkoškrkos@cesnet.cz
<ul></ul><p>Rajmund Hruska wrote in <a href="#note-14">#note-14</a>:</p>
<blockquote>
<p>I can't fully remove <code>sqlalchemy</code> module because <code>sqlalchemy.exc.OperationalError</code> is used and <code>exc</code> module is <a href="https://github.com/pallets/flask-sqlalchemy/issues/764" class="external">not present in the <code>flask_sqlalchemy</code> module</a>.</p>
</blockquote>
<p>I have found:</p>
<pre>
flask_sqlalchemy.orm.exc.sa_exc.OperationalError
</pre>
<p>Not thoroughly tested.</p>
<p>Also, there seems to be proper support for model definition:<br /><a class="external" href="https://flask-sqlalchemy.palletsprojects.com/en/2.x/models/">https://flask-sqlalchemy.palletsprojects.com/en/2.x/models/</a></p>
<p>Lastly, I would advise to redo the patches from scratch, the commit history is getting hard to understand.</p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=357612021-10-11T14:08:26ZRajmund Hruška
<ul></ul><p>Radko Krkoš wrote in <a href="#note-15">#note-15</a>:</p>
<blockquote>
<p>Rajmund Hruska wrote in <a href="#note-14">#note-14</a>:</p>
<blockquote>
<p>I can't fully remove <code>sqlalchemy</code> module because <code>sqlalchemy.exc.OperationalError</code> is used and <code>exc</code> module is <a href="https://github.com/pallets/flask-sqlalchemy/issues/764" class="external">not present in the <code>flask_sqlalchemy</code> module</a>.</p>
</blockquote>
<p>I have found:</p>
<p>[...]</p>
<p>Not thoroughly tested.</p>
</blockquote>
<p>I tried running the following code:<br /><pre><code class="python syntaxhl"><span class="kn">import</span> <span class="nn">flask_sqlalchemy</span>
<span class="kn">import</span> <span class="nn">sqlalchemy</span>
<span class="k">def</span> <span class="nf">func</span><span class="p">():</span>
<span class="k">raise</span> <span class="n">sqlalchemy</span><span class="p">.</span><span class="n">exc</span><span class="p">.</span><span class="n">OperationalError</span><span class="p">(</span><span class="s">'Test'</span><span class="p">,</span> <span class="n">orig</span><span class="o">=</span><span class="bp">None</span><span class="p">,</span> <span class="n">params</span><span class="o">=</span><span class="bp">None</span><span class="p">)</span>
<span class="k">def</span> <span class="nf">func2</span><span class="p">():</span>
<span class="k">try</span><span class="p">:</span>
<span class="n">func</span><span class="p">()</span>
<span class="k">except</span> <span class="n">flask_sqlalchemy</span><span class="p">.</span><span class="n">orm</span><span class="p">.</span><span class="n">exc</span><span class="p">.</span><span class="n">sa_exc</span><span class="p">.</span><span class="n">OperationalError</span><span class="p">:</span>
<span class="k">print</span><span class="p">(</span><span class="s">'caught'</span><span class="p">)</span>
<span class="n">func2</span><span class="p">()</span>
</code></pre><br />After executing the code, 'caught' was printed on the screen so I assume it is possible to use <code>flask_sqlalchemy.orm.exc.sa_exc.OperationalError</code> instead of <code>sqlalchemy.exc.OperationalError</code>.</p>
<blockquote>
<p>Also, there seems to be proper support for model definition:<br /><a class="external" href="https://flask-sqlalchemy.palletsprojects.com/en/2.x/models/">https://flask-sqlalchemy.palletsprojects.com/en/2.x/models/</a></p>
</blockquote>
<p>I will look into that in more detail.</p>
<blockquote>
<p>Lastly, I would advise to redo the patches from scratch, the commit history is getting hard to understand.</p>
</blockquote>
<p>I will create a new branch.</p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=357792021-10-19T14:35:46ZRajmund Hruška
<ul></ul><p>I checked the <code>eventstorage.py</code> service, which uses Psycopg2. I don't have any experience with Psycopg2, I only read the official <a href="https://www.psycopg.org/docs/usage.html" class="external">documentation</a>. From my point of view, the Psycopg2 is used correctly and I wouldn't change anything there.</p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=357802021-10-19T14:39:44ZRadko Krkoškrkos@cesnet.cz
<ul></ul><p>Rajmund Hruska wrote in <a href="#note-17">#note-17</a>:</p>
<blockquote>
<p>I checked the <code>eventstorage.py</code> service, which uses Psycopg2. I don't have any experience with Psycopg2, I only read the official <a href="https://www.psycopg.org/docs/usage.html" class="external">documentation</a>. From my point of view, the Psycopg2 is used correctly and I wouldn't change anything there.</p>
</blockquote>
<p>Yes, this is about an idiosyncrasy of the <code>SQLAlchemy</code> ORM. With <code>psycopg2</code>, the transactions are managed differently (manually), so there should be no gotchas there.</p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=357812021-10-19T14:43:55ZRajmund Hruška
<ul></ul><p>Radko Krkoš wrote in <a href="#note-18">#note-18</a>:</p>
<blockquote>
<p>Rajmund Hruska wrote in <a href="#note-17">#note-17</a>:</p>
<blockquote>
<p>I checked the <code>eventstorage.py</code> service, which uses Psycopg2. I don't have any experience with Psycopg2, I only read the official <a href="https://www.psycopg.org/docs/usage.html" class="external">documentation</a>. From my point of view, the Psycopg2 is used correctly and I wouldn't change anything there.</p>
</blockquote>
<p>Yes, this is about an idiosyncrasy of the <code>SQLAlchemy</code> ORM. With <code>psycopg2</code>, the transactions are managed differently (manually), so there should be no gotchas there.</p>
</blockquote>
<p>I thought so but I checked <code>psycopg2</code> because it was mentioned in the title and description of the issue. Anyway, at least I understand it better now <span class="wiking smiley smiley-smiley" title=":)"></span></p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=359252021-12-15T08:03:50ZRajmund Hruška
<ul><li><strong>To be discussed</strong> changed from <i>No</i> to <i>Yes</i></li></ul> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=359302022-01-03T16:31:03ZRajmund Hruška
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Feedback</i></li><li><strong>% Done</strong> changed from <i>50</i> to <i>100</i></li></ul><p>So, in <a class="changeset" title="Feature: Use SQLAlchemy from flask_sqlalchemy. (Redmine issue: #6205)" href="https://homeproj.cesnet.cz/projects/mentat/repository/mentat-ng/revisions/ed1b8f239ee67b9af9899b2d4ebe35574e87de0c">ed1b8f23</a> I started using <code>flask_sqlalchemy</code> for sessions in <code>lib/mentat</code>. In <code>lib/hawat</code> the <code>flask_sqlalchemy</code> is already used. And <code>lib/vial</code> will be merged into <code>lib/hawat</code>. I guess the issue is resolved now, isn't it?</p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=359362022-01-04T05:47:18ZRadko Krkoškrkos@cesnet.cz
<ul></ul><p>Rajmund Hruska wrote in <a href="#note-21">#note-21</a>:</p>
<blockquote>
<p>So, in <a class="changeset" title="Feature: Use SQLAlchemy from flask_sqlalchemy. (Redmine issue: #6205)" href="https://homeproj.cesnet.cz/projects/mentat/repository/mentat-ng/revisions/ed1b8f239ee67b9af9899b2d4ebe35574e87de0c">ed1b8f23</a> I started using <code>flask_sqlalchemy</code> for sessions in <code>lib/mentat</code>. In <code>lib/hawat</code> the <code>flask_sqlalchemy</code> is already used. And <code>lib/vial</code> will be merged into <code>lib/hawat</code>. I guess the issue is resolved now, isn't it?</p>
</blockquote>
<p>From a quick glance, not all issues from the previous review have been dealt with. At least I can see the <code>sqlalchemy.exc.OperationalError</code> one unaddressed.<br />In addition, I do not like the inconsistency of going to full path in code on lines 31 & 44 (after diff) while going in the other direction on lines 32 & 53. Why?</p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=359372022-01-04T08:34:23ZRajmund Hruška
<ul></ul><p>Radko Krkoš wrote in <a href="#note-22">#note-22</a>:</p>
<blockquote>
<p>Rajmund Hruska wrote in <a href="#note-21">#note-21</a>:</p>
<blockquote>
<p>So, in <a class="changeset" title="Feature: Use SQLAlchemy from flask_sqlalchemy. (Redmine issue: #6205)" href="https://homeproj.cesnet.cz/projects/mentat/repository/mentat-ng/revisions/ed1b8f239ee67b9af9899b2d4ebe35574e87de0c">ed1b8f23</a> I started using <code>flask_sqlalchemy</code> for sessions in <code>lib/mentat</code>. In <code>lib/hawat</code> the <code>flask_sqlalchemy</code> is already used. And <code>lib/vial</code> will be merged into <code>lib/hawat</code>. I guess the issue is resolved now, isn't it?</p>
</blockquote>
<p>From a quick glance, not all issues from the previous review have been dealt with. At least I can see the <code>sqlalchemy.exc.OperationalError</code> one unaddressed.</p>
</blockquote>
<p>I think we talked about that in one of the meetings but I don't remember the reason why I didn't the <code>OperationalError</code> from <code>flask_sqlalchemy</code>. I guess I can use the other one, as it seems to be working anyway (<a href="#note-16">#note-16</a>).</p>
<blockquote>
<p>In addition, I do not like the inconsistency of going to full path in code on lines 31 & 44 (after diff) while going in the other direction on lines 32 & 53. Why?</p>
</blockquote>
<p>There wasn't really any particular reason at all it was just how it was convenient at the time. I only needed the <code>OperationalError</code> in lines 32 & 53 and not full <code>sqlalchemy</code> as it was before diff. And I needed multiple objects from <code>flask_sqlalchemy</code> (lines 76, 78).</p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=359382022-01-04T09:12:47ZRadko Krkoškrkos@cesnet.cz
<ul></ul><p>Rajmund Hruska wrote in <a href="#note-23">#note-23</a>:</p>
<blockquote>
<p>Radko Krkoš wrote in <a href="#note-22">#note-22</a>:</p>
<blockquote>
<p>Rajmund Hruska wrote in <a href="#note-21">#note-21</a>:</p>
<blockquote>
<p>So, in <a class="changeset" title="Feature: Use SQLAlchemy from flask_sqlalchemy. (Redmine issue: #6205)" href="https://homeproj.cesnet.cz/projects/mentat/repository/mentat-ng/revisions/ed1b8f239ee67b9af9899b2d4ebe35574e87de0c">ed1b8f23</a> I started using <code>flask_sqlalchemy</code> for sessions in <code>lib/mentat</code>. In <code>lib/hawat</code> the <code>flask_sqlalchemy</code> is already used. And <code>lib/vial</code> will be merged into <code>lib/hawat</code>. I guess the issue is resolved now, isn't it?</p>
</blockquote>
<p>From a quick glance, not all issues from the previous review have been dealt with. At least I can see the <code>sqlalchemy.exc.OperationalError</code> one unaddressed.</p>
</blockquote>
<p>I think we talked about that in one of the meetings but I don't remember the reason why I didn't the <code>OperationalError</code> from <code>flask_sqlalchemy</code>. I guess I can use the other one, as it seems to be working anyway (<a href="#note-16">#note-16</a>).</p>
<blockquote>
<p>In addition, I do not like the inconsistency of going to full path in code on lines 31 & 44 (after diff) while going in the other direction on lines 32 & 53. Why?</p>
</blockquote>
<p>There wasn't really any particular reason at all it was just how it was convenient at the time. I only needed the <code>OperationalError</code> in lines 32 & 53 and not full <code>sqlalchemy</code> as it was before diff. And I needed multiple objects from <code>flask_sqlalchemy</code> (lines 76, 78).</p>
</blockquote>
<p>OK.</p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=359582022-01-06T12:49:03ZPavel Káchaph@cesnet.cz
<ul></ul><p>As the most important aim of this task was to get rid transaction dancing kludge to sidestep bare SQLAlchemy idiosyncracies, I guess we're done here - time for polishing and merging.</p>
<p>There are still cases of questionable *SQLAlchemy usage, I've created another ticket for long winter evenings: <a class="issue tracker-5 status-8 priority-4 priority-default" title="Task: Review SQLAlchemy vs Flask-SQLAlchemy consolidation options (Deferred)" href="https://homeproj.cesnet.cz/issues/7545">#7545</a>.</p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=359642022-01-12T15:08:27ZRajmund Hruška
<ul><li><strong>Status</strong> changed from <i>Feedback</i> to <i>In Review</i></li><li><strong>To be discussed</strong> deleted (<del><i>Yes</i></del>)</li></ul><p>Merged into <strong>devel</strong>.</p> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=361152022-03-22T17:27:55ZPavel Káchaph@cesnet.cz
<ul><li><strong>Status</strong> changed from <i>In Review</i> to <i>Closed</i></li></ul> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=365292023-03-29T13:53:17ZJakub Judiny
<ul><li><strong>Related to</strong> <i><a class="issue tracker-1 status-2 priority-4 priority-default" href="/issues/7619">Bug #7619</a>: Flask Deprecation warnings</i> added</li></ul> Mentat - Task #6205: Review encapsulation of SQLAlchemy and Psycopg2 DBALshttps://homeproj.cesnet.cz/issues/6205?journal_id=367092023-05-04T14:38:14ZJakub Judiny
<ul><li><strong>Related to</strong> <i><a class="issue tracker-3 status-5 priority-4 priority-default closed" href="/issues/7642">Support #7642</a>: Upgrade Flask-SQLAlchemy</i> added</li></ul>