22827 - Add sql-versioning library and versioning switching logic #3024

chenhongjing · 2024-10-10T18:58:59Z

Description of changes:

Add sql-versioning library as new db versioning, including docstring, doc, tests etc.
Import sql-versioning library to legal-api
Add db versioning switching logic with feature flag to legal-api
Update db models
Tweak current db versioning to use custom versioning_manager and Transaction model
Update versioning_manager uses across components

Note that

All the coloured debugging messages are kept in the code for further development
Currently versioning switching logic is disabled (setup function is commented out), and only old db versioning is enabled

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the lear license (Apache 2.0).

gitguardian · 2024-10-10T18:59:46Z

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.

🔎 Detected hardcoded secret in your pull request

GitGuardian id	GitGuardian status	Secret	Commit	Filename
9442085	Triggered	Generic Password	`685ee7a`	python/common/sql-versioning/tests/docker-compose.yml	View secret

🛠 Guidelines to remediate hardcoded secrets

Understand the implications of revoking this secret by investigating where it is used in your code.
Replace and store your secret safely. Learn here the best practices.
Revoke and rotate this secret.
If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider

following these best practices for managing and storing secrets including API keys and other credentials
install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.}

jobs/furnishings/requirements.txt

chenhongjing · 2024-10-11T15:27:25Z

legal-api/src/legal_api/models/corp_type.py

@@ -20,7 +20,6 @@
 class CorpType(db.Model):  # pylint: disable=too-many-instance-attributes
    """This class manages the corp type."""

-    __versioned__ = {}


Does corp_type have version table? I don't find it so I remove its old versioning.

couldn't find it in DEV db on my side

chenhongjing · 2024-10-11T15:37:24Z

legal-api/src/legal_api/models/db.py

-# make_versioned(user_cls=None, plugins=[FlaskPlugin()])
-make_versioned(user_cls=None)
+
+class Transaction(db.Model):


Introduce custom Transaction model to solve FK related issues between filings and transaction tables when we completely remove old versioning and only new versioning exists. Put it in db.py file for now to avoid the import issue when starting up the flask app.

I make old and new versionings use this custom model to make sure there's no conflicts when both of them exist in the code, though they have their own transaction model. And the removal of old versioning will be clean in the future and the core logic of new versioning won't be touched then.

chenhongjing · 2024-10-11T15:43:21Z

python/common/sql-versioning/sql_versioning/versioning.py

Let me know if I need to split this into multiple files.

Should be fine as one file I think

chenhongjing · 2024-10-11T15:47:26Z

python/common/sql-versioning/sql_versioning/versioning.py

+        if not _is_session_modified(session):
+            print('\033[31mThere is no modified object in this session.\033[0m')
+            return


The logic is updated after the experimental code to make sure that no new transaction entry will be created if the versioned object is not "actually" modified..

chenhongjing · 2024-10-11T15:55:45Z

python/common/sql-versioning/tests/docker-compose.yml

A test postgres db container will be started automatically during testing.

argush3 · 2024-10-16T17:16:19Z

legal-api/flags.json

+        "db-versioning": {
+            "legal-api": false,
+            "emailer": false,
+            "filer": false,
+            "entity-bn": false,
+            "digital-credentials": false,
+            "dissolutions-job": false,
+            "furnishings-job": false,
+            "emailer-reminder-job": false,
+            "future-effective-job": false,
+            "update-colin-filings-job": false,
+            "update-legal-filings-job": false
        }


do we have this flag setup for each environment in LD already?

Not yet, and the related setup_versioning() function is commented out so there's no impact. But I'll set them up now.

Done. The flag is off for all env.

Current on value:

{ "legal-api": false, "emailer": false, "filer": false, "entity-bn": false, "digital-credentials": false, "dissolutions-job": false, "furnishings-job": false, "emailer-reminder-job": false, "future-effective-job": false, "update-colin-filings-job": false, "update-legal-filings-job": false }

Current off value:

{}

legal-api/src/legal_api/models/db.py

argush3 · 2024-10-16T19:02:26Z

legal-api/src/legal_api/models/db.py

+# TODO: enable versioning switching
+# it should be called before data model initialzed, otherwise, old versioning doesn't work properly
+# setup_versioning()


so this will be uncommented as a part of the work in #22942 right?

JazzarKarim

From what I can see, looks great!

I may have missed something as the work here is kind of brand new to myself. I'm up to speed now though after going through this PR.

argush3 · 2024-10-17T14:43:19Z

python/common/sql-versioning/tests/test_versioning.py

+    # the following operations should not result in new versioned records for native sqlalchemy session
+    session.add(user)
+    session.commit()


thanks, very thorough to test this scenario too!

argush3 · 2024-10-17T14:47:24Z

python/common/sql-versioning/tests/test_versioning.py

These tests are great!

Will be a great starting point to verify the versioning library and also if we ever need to add other tests.

I don't believe we have any CI for this and I think that's fine for now.

But can you provide a screenshot of the test results when you run things locally?

FYI the test results at local env, and REAME.md has the setup instructions for this work so that anyone can play around it and test it easily. (think I had better update the application name to align with the one in sbc-connect-common repo later).

argush3 · 2024-10-17T15:08:04Z

queue_services/entity-digital-credentials/requirements.txt

+# git+https://github.com/bcgov/lear.git#egg=legal_api&subdirectory=legal-api
+git+https://github.com/chenhongjing/lear.git@22827-sql-versioning#egg=legal_api&subdirectory=legal-api
+git+https://github.com/chenhongjing/lear.git@22827-sql-versioning#egg=sql-versioning&subdirectory=python/common/sql-versioning


So when we are ready to merge, what do all the queue service requirements files look like?

Will it just be a reference to git+https://github.com/bcgov/lear.git#egg=legal_api&subdirectory=legal-api?

We need to add a new dependency - sql-versioning (reference: failed CI), so it will be

git+https://github.com/bcgov/lear.git#egg=legal_api&subdirectory=legal-api git+https://github.com/bcgov/lear.git#egg=sql-versioning&subdirectory=python/common/sql-versioning

argush3 · 2024-10-17T15:11:17Z

Great work!

Will approve once some of the comments I've added have been addressed.

Also, think you'll need to rebase as there are some conflicts with main

Signed-off-by: chenhongjing <[email protected]>

argush3

Approving with assumption following will be addressed:

conflict with main
dependencies in requirements.txt files will be updated as appropriate

Signed-off-by: chenhongjing <[email protected]>

Signed-off-by: Hongjing Chen <[email protected]>

leodube-aot

Looks good to me! Lots of learning for me going through this

Signed-off-by: Hongjing Chen <[email protected]>

sonarcloud · 2024-10-21T14:55:22Z

Quality Gate passed

Issues
18 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

chenhongjing · 2024-10-21T14:56:38Z

Just add a test of rollback scenario for sql-versioning, and no significant changes. Going to merge this pr after basic checks are done.

chenhongjing self-assigned this Oct 10, 2024

chenhongjing force-pushed the 22827-sql-versioning branch from 6ecd529 to 3d154e6 Compare October 10, 2024 19:23

chenhongjing commented Oct 11, 2024

View reviewed changes

jobs/furnishings/requirements.txt Outdated Show resolved Hide resolved

chenhongjing commented Oct 11, 2024

View reviewed changes

chenhongjing marked this pull request as ready for review October 11, 2024 16:00

chenhongjing requested review from argush3, kzdev420, vysakh-menon-aot, JazzarKarim, leodube-aot, AimeeGao and eason-pan-bc October 11, 2024 16:00

argush3 reviewed Oct 16, 2024

View reviewed changes

legal-api/src/legal_api/models/db.py Show resolved Hide resolved

JazzarKarim reviewed Oct 16, 2024

View reviewed changes

legal-api/src/legal_api/models/db.py Show resolved Hide resolved

argush3 reviewed Oct 16, 2024

View reviewed changes

JazzarKarim approved these changes Oct 16, 2024

View reviewed changes

argush3 reviewed Oct 17, 2024

View reviewed changes

22827 - add sql-versioning library & versioning switching logic

685ee7a

Signed-off-by: chenhongjing <[email protected]>

argush3 approved these changes Oct 17, 2024

View reviewed changes

chenhongjing and others added 2 commits October 17, 2024 08:36

update requirements.txt

a630e64

Signed-off-by: chenhongjing <[email protected]>

fix linting

117154f

Signed-off-by: Hongjing Chen <[email protected]>

chenhongjing added 9 commits October 17, 2024 08:37

temporarily point to another branch dependencies

474af62

Signed-off-by: Hongjing Chen <[email protected]>

fix linting & add dependencies

ff8194d

Signed-off-by: Hongjing Chen <[email protected]>

fix linting

f209423

Signed-off-by: Hongjing Chen <[email protected]>

fix linting

5d53988

Signed-off-by: Hongjing Chen <[email protected]>

update sql-versioning

2729320

Signed-off-by: Hongjing Chen <[email protected]>

add TODO for sql-versioning

a1be15f

Signed-off-by: Hongjing Chen <[email protected]>

small tweaks

24d98d8

Signed-off-by: Hongjing Chen <[email protected]>

update README.md of sql-versioning

668078f

Signed-off-by: Hongjing Chen <[email protected]>

update dependencies

beb7aec

Signed-off-by: Hongjing Chen <[email protected]>

chenhongjing force-pushed the 22827-sql-versioning branch from 2c69570 to beb7aec Compare October 17, 2024 15:45

leodube-aot approved these changes Oct 17, 2024

View reviewed changes

chenhongjing added the DO NOT MERGE label Oct 17, 2024

add test for rollback

56235b3

Signed-off-by: Hongjing Chen <[email protected]>

chenhongjing removed the DO NOT MERGE label Oct 21, 2024

chenhongjing merged commit 4b41a96 into bcgov:main Oct 21, 2024
6 of 23 checks passed

chenhongjing mentioned this pull request Oct 21, 2024

22827 - Update requirement files to include sql-versioning #3031

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

22827 - Add sql-versioning library and versioning switching logic #3024

22827 - Add sql-versioning library and versioning switching logic #3024

chenhongjing commented Oct 10, 2024 •

edited

Loading

gitguardian bot commented Oct 10, 2024 •

edited

Loading

chenhongjing Oct 11, 2024

eason-pan-bc Oct 16, 2024

chenhongjing Oct 11, 2024 •

edited

Loading

chenhongjing Oct 11, 2024

argush3 Oct 17, 2024

chenhongjing Oct 11, 2024

chenhongjing Oct 11, 2024

argush3 Oct 16, 2024

chenhongjing Oct 16, 2024

chenhongjing Oct 16, 2024

argush3 Oct 16, 2024

chenhongjing Oct 16, 2024

JazzarKarim left a comment

argush3 Oct 17, 2024

argush3 Oct 17, 2024

chenhongjing Oct 17, 2024

argush3 Oct 17, 2024 •

edited

Loading

chenhongjing Oct 17, 2024

argush3 commented Oct 17, 2024

argush3 left a comment

leodube-aot left a comment

sonarcloud bot commented Oct 21, 2024

chenhongjing commented Oct 21, 2024

22827 - Add sql-versioning library and versioning switching logic #3024

22827 - Add sql-versioning library and versioning switching logic #3024

Conversation

chenhongjing commented Oct 10, 2024 • edited Loading

gitguardian bot commented Oct 10, 2024 • edited Loading

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chenhongjing Oct 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JazzarKarim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

argush3 Oct 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

argush3 commented Oct 17, 2024

argush3 left a comment

Choose a reason for hiding this comment

leodube-aot left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Oct 21, 2024

Quality Gate passed

chenhongjing commented Oct 21, 2024

chenhongjing commented Oct 10, 2024 •

edited

Loading

gitguardian bot commented Oct 10, 2024 •

edited

Loading

chenhongjing Oct 11, 2024 •

edited

Loading

argush3 Oct 17, 2024 •

edited

Loading