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.

Sign up for GitHub

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

22948 - update emailer to use new db versioning #3092

Merged
merged 16 commits into from
Nov 28, 2024
Merged

22948 - update emailer to use new db versioning #3092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
3081a36
enable emailer db versioning feature flag
eason-pan-bc Nov 22, 2024
95dd3b2
update to use versioning proxy
eason-pan-bc Nov 22, 2024
660dc68
add service name to config.py
eason-pan-bc Nov 23, 2024
ec76671
initialize new versioning for emailer and add tracker feature flag
eason-pan-bc Nov 23, 2024
c3a23fc
add tracker feature flag and clean up debugging config
eason-pan-bc Nov 23, 2024
b1775c4
clean up code
eason-pan-bc Nov 23, 2024
703876c
update dev purpose sql-versioning installation branch and initialize …
eason-pan-bc Nov 25, 2024
e377ba2
update requirements and clean up code
eason-pan-bc Nov 25, 2024
4871305
fix linting
eason-pan-bc Nov 25, 2024
2582c73
fix some typos
eason-pan-bc Nov 25, 2024
ce1730e
remove unnecessary initialization
eason-pan-bc Nov 26, 2024
b60700b
update flags
eason-pan-bc Nov 26, 2024
610956b
change back sql-versioning pack to use main
eason-pan-bc Nov 26, 2024
6812e79
add comments to configs
eason-pan-bc Nov 26, 2024
82e42c8
adjust tracker to use emailer as service name
eason-pan-bc Nov 26, 2024
8c0c062
update flags
eason-pan-bc Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion legal-api/flags.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
},
"db-versioning": {
"legal-api": true,
"emailer": false,
"emailer": true,
"filer": false,
"entity-bn": false,
"digital-credentials": false,
Expand Down
2 changes: 1 addition & 1 deletion legal-api/requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,4 @@ reportlab==3.6.12
html-sanitizer==2.4.1
lxml==5.2.2
git+https://github.com/bcgov/[email protected]#egg=registry_schemas
git+https://github.com/bcgov/lear.git#egg=sql-versioning&subdirectory=python/common/sql-versioning
git+https://github.com/bcgov/lear.git#egg=sql-versioning&subdirectory=python/common/sql-versioning
6 changes: 3 additions & 3 deletions legal-api/src/legal_api/models/db.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""Create SQLAlchenmy and Schema managers.
"""Create SQLAlchemy and Schema managers.

These will get initialized by the application using the models
"""
Expand Down Expand Up @@ -267,7 +267,7 @@ def version_class(cls, session, obj):

@debug
def setup_versioning():
"""Set up and initialize versioining switching.
"""Set up and initialize versioning switching.

:return: None
"""
Expand All @@ -287,7 +287,7 @@ def clear_transaction(session, transaction):


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

# make_versioned(user_cls=None, manager=versioning_manager)
2 changes: 1 addition & 1 deletion python/common/sql-versioning/sql_versioning/versioning.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def _is_obj_modified(obj):
def _is_session_modified(session):
"""Check if the session contains modified versioned objects.

:param session: The database sesseion instance.
:param session: The database session instance.
:return: True if the session contains modified versioned objects, otherwise False.
"""
for obj in versioned_objects(session):
Expand Down
15 changes: 14 additions & 1 deletion queue_services/entity-emailer/flags.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
{
"flagValues": {
"disable-specific-service-provider": true
"disable-specific-service-provider": true,
"db-versioning": {
"legal-api": true,
"emailer": true,
"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
}
}
}
9 changes: 7 additions & 2 deletions queue_services/entity-emailer/requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ rsa==4.7.2
semver==2.13.0
sentry-sdk==1.20.0
six==1.15.0
SQLAlchemy==1.3.24
SQLAlchemy==1.4.44
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

update the SQLAlchemy version to make it compatible with the custom sql-versioning

SQLAlchemy-Continuum==1.3.13
SQLAlchemy-Utils==0.37.1
strict-rfc3339==0.7
Expand All @@ -79,6 +79,11 @@ Werkzeug==1.0.1
yarl==1.8.2
zipp==3.15.0
git+https://github.com/bcgov/[email protected]#egg=registry_schemas
git+https://github.com/bcgov/lear.git#egg=legal_api&subdirectory=legal-api

# install updated legal-api from feature branch for development,
# TODO - change it back when the db versioning feature branch merged to main
git+https://github.com/bcgov/lear.git@feature-db-versioning#egg=legal_api&subdirectory=legal-api
# git+https://github.com/bcgov/lear.git#egg=legal_api&subdirectory=legal-api
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@leodube-aot how about the legal-api package, installing from the feature branch?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yup we'll be installing the legal-api package from the feature branch


git+https://github.com/bcgov/lear.git#egg=entity_queue_common&subdirectory=queue_services/common
git+https://github.com/bcgov/lear.git#egg=sql-versioning&subdirectory=python/common/sql-versioning
2 changes: 2 additions & 0 deletions queue_services/entity-emailer/src/entity_emailer/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ class _Config(): # pylint: disable=too-few-public-methods
Used as the base for all the other configurations.
"""

# used to identify versioning flag
SERVICE_NAME = 'emailer'
PROJECT_ROOT = os.path.abspath(os.path.dirname(__file__))

MSG_RETRY_NUM = int(os.getenv('MSG_RETRY_NUM', '5'))
Expand Down
6 changes: 3 additions & 3 deletions queue_services/entity-emailer/src/entity_emailer/worker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
from entity_queue_common.service import QueueServiceManager
from entity_queue_common.service_utils import EmailException, QueueException, logger
from flask import Flask
from legal_api import db
from legal_api import db, init_db # noqa:F401,I001;pylint:disable=unused-import;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Didn't remove the db as in test when mocking, the db attribute is needed for worker. As experimented, removing it caused unit-tests initialization errors. https://github.com/bcgov/lear/blob/feature-db-versioning/queue_services/entity-emailer/tests/conftest.py#L322C1-L322C42

Copy link
Collaborator

Choose a reason for hiding this comment

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

What's interesting is that you can remove this unused db import along with the monkeypatch.setattr(worker, 'db', db) line, and the unit tests work fine again. I think we should leave it as is for now. It's been in the code for 3+ years to no ill effect, and we want to limit the number of changes we merge back into main.

from legal_api.models import Filing, Furnishing
from legal_api.services.bootstrap import AccountService
from legal_api.services.flags import Flags
Expand Down Expand Up @@ -65,14 +65,14 @@
)

from .message_tracker import tracker as tracker_util

# noqa:I003

qsm = QueueServiceManager() # pylint: disable=invalid-name
flags = Flags() # pylint: disable=invalid-name
APP_CONFIG = config.get_named_config(os.getenv('DEPLOYMENT_ENV', 'production'))
FLASK_APP = Flask(__name__)
FLASK_APP.config.from_object(APP_CONFIG)
db.init_app(FLASK_APP)
init_db(FLASK_APP)

if FLASK_APP.config.get('LD_SDK_KEY', None):
flags.init_app(FLASK_APP)
Expand Down
42 changes: 17 additions & 25 deletions queue_services/entity-emailer/tests/unit/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from unittest.mock import Mock

from legal_api.models import Batch, Business, Filing, Furnishing, Party, PartyRole, RegistrationBootstrap, User
from legal_api.models.db import versioning_manager
from legal_api.models.db import VersioningProxy
from registry_schemas.example_data import (
AGM_EXTENSION,
AGM_LOCATION_CHANGE,
Expand Down Expand Up @@ -127,9 +127,8 @@ def prep_incorp_filing(session, identifier, payment_id, option, legal_type=None)
filing.payment_completion_date = filing.filing_date
filing.save()
if option in ['COMPLETED', 'bn']:
uow = versioning_manager.unit_of_work(session)
transaction = uow.create_transaction(session)
filing.transaction_id = transaction.id
transaction_id = VersioningProxy.get_transaction_id(session())
filing.transaction_id = transaction_id
filing.save()
return filing

Expand Down Expand Up @@ -193,9 +192,8 @@ def prep_registration_filing(session, identifier, payment_id, option, legal_type
filing.payment_completion_date = filing.filing_date
filing.save()
if option in ['COMPLETED']:
uow = versioning_manager.unit_of_work(session)
transaction = uow.create_transaction(session)
filing.transaction_id = transaction.id
transaction_id = VersioningProxy.get_transaction_id(session())
filing.transaction_id = transaction_id
filing.save()
return filing

Expand Down Expand Up @@ -471,9 +469,8 @@ def prep_maintenance_filing(session, identifier, payment_id, status, filing_type

filing.save()
if status == 'COMPLETED':
uow = versioning_manager.unit_of_work(session)
transaction = uow.create_transaction(session)
filing.transaction_id = transaction.id
transaction_id = VersioningProxy.get_transaction_id(session())
filing.transaction_id = transaction_id
filing.save()

return filing
Expand All @@ -493,9 +490,8 @@ def prep_incorporation_correction_filing(session, business, original_filing_id,
filing.payment_completion_date = filing.filing_date
filing.save()
if option in ['COMPLETED']:
uow = versioning_manager.unit_of_work(session)
transaction = uow.create_transaction(session)
filing.transaction_id = transaction.id
transaction_id = VersioningProxy.get_transaction_id(session())
filing.transaction_id = transaction_id
filing.save()
return filing

Expand Down Expand Up @@ -600,9 +596,8 @@ def prep_cp_special_resolution_correction_filing(session, business, original_fil
filing._meta_data = {'correction': {'uploadNewRules': True, 'toLegalName': True}}
filing.save()
if option in ['COMPLETED']:
uow = versioning_manager.unit_of_work(session)
transaction = uow.create_transaction(session)
filing.transaction_id = transaction.id
transaction_id = VersioningProxy.get_transaction_id(session())
filing.transaction_id = transaction_id
filing.save()
return filing

Expand All @@ -628,9 +623,8 @@ def prep_cp_special_resolution_correction_upload_memorandum_filing(session, busi
filing._meta_data = {'correction': {'uploadNewMemorandum': True}}
filing.save()
if option in ['COMPLETED']:
uow = versioning_manager.unit_of_work(session)
transaction = uow.create_transaction(session)
filing.transaction_id = transaction.id
transaction_id = VersioningProxy.get_transaction_id(session())
filing.transaction_id = transaction_id
filing.save()
return filing

Expand Down Expand Up @@ -663,9 +657,8 @@ def prep_amalgamation_filing(session, identifier, payment_id, option, legal_name
filing.payment_completion_date = filing.filing_date
filing.save()
if option in [Filing.Status.COMPLETED.value, 'bn']:
uow = versioning_manager.unit_of_work(session)
transaction = uow.create_transaction(session)
filing.transaction_id = transaction.id
transaction_id = VersioningProxy.get_transaction_id(session())
filing.transaction_id = transaction_id
filing.save()
return filing

Expand All @@ -691,9 +684,8 @@ def prep_continuation_in_filing(session, identifier, payment_id, option):
filing.payment_completion_date = filing.filing_date
filing.save()
if option in [Filing.Status.COMPLETED.value, 'bn']:
uow = versioning_manager.unit_of_work(session)
transaction = uow.create_transaction(session)
filing.transaction_id = transaction.id
transaction_id = VersioningProxy.get_transaction_id(session())
filing.transaction_id = transaction_id
filing.save()
return filing

Expand Down
4 changes: 3 additions & 1 deletion queue_services/entity-emailer/tracker/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ class _Config(): # pylint: disable=too-few-public-methods
"""

# PROJECT_ROOT = os.path.abspath(os.path.dirname(__file__))


# used to identify versioning flag
SERVICE_NAME = 'emailer'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As per discussion using flag emailer for the tracker. #3092 (comment)

SQLALCHEMY_TRACK_MODIFICATIONS = False

# POSTGRESQL
Expand Down