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.

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

Remove AUTOINITIALIZE env variable #1824

Merged
merged 1 commit into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 14 additions & 20 deletions src/palace/manager/api/app.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import logging
import os

import flask_babel
from flask import request
Expand All @@ -17,7 +16,7 @@
PalaceXrayProfiler,
)
from palace.manager.core.app_server import ErrorHandler
from palace.manager.service.container import Services, container_instance
from palace.manager.service.container import container_instance

Check warning on line 19 in src/palace/manager/api/app.py

View check run for this annotation

Codecov / codecov/patch

src/palace/manager/api/app.py#L19

Added line #L19 was not covered by tests
from palace.manager.sqlalchemy.flask_sqlalchemy_session import flask_scoped_session
from palace.manager.sqlalchemy.model.key import Key, KeyType
from palace.manager.sqlalchemy.session import SessionManager
Expand Down Expand Up @@ -66,24 +65,19 @@
).value


def initialize_circulation_manager(container: Services):
if os.environ.get("AUTOINITIALIZE") == "False":
# It's the responsibility of the importing code to set app.manager
# appropriately.
pass
else:
if getattr(app, "manager", None) is None:
try:
app.manager = CirculationManager(app._db)
except Exception:
logging.exception("Error instantiating circulation manager!")
raise
# Make sure that any changes to the database (as might happen
# on initial setup) are committed before continuing.
app.manager._db.commit()
def initialize_circulation_manager():

Check warning on line 68 in src/palace/manager/api/app.py

View check run for this annotation

Codecov / codecov/patch

src/palace/manager/api/app.py#L68

Added line #L68 was not covered by tests
if getattr(app, "manager", None) is None:
try:
app.manager = CirculationManager(app._db)
except Exception:
logging.exception("Error instantiating circulation manager!")
raise

Check warning on line 74 in src/palace/manager/api/app.py

View check run for this annotation

Codecov / codecov/patch

src/palace/manager/api/app.py#L70-L74

Added lines #L70 - L74 were not covered by tests
# Make sure that any changes to the database (as might happen
# on initial setup) are committed before continuing.
app.manager._db.commit()

Check warning on line 77 in src/palace/manager/api/app.py

View check run for this annotation

Codecov / codecov/patch

src/palace/manager/api/app.py#L77

Added line #L77 was not covered by tests

# setup the cache data object
CachedData.initialize(app._db)
# setup the cache data object
CachedData.initialize(app._db)

Check warning on line 80 in src/palace/manager/api/app.py

View check run for this annotation

Codecov / codecov/patch

src/palace/manager/api/app.py#L80

Added line #L80 was not covered by tests


def initialize_database():
Expand Down Expand Up @@ -113,6 +107,6 @@
app.register_error_handler(Exception, error_handler.handle)

# Initialize the circulation manager
initialize_circulation_manager(container)
initialize_circulation_manager()

Check warning on line 110 in src/palace/manager/api/app.py

View check run for this annotation

Codecov / codecov/patch

src/palace/manager/api/app.py#L110

Added line #L110 was not covered by tests
initialize_admin()
return app
36 changes: 34 additions & 2 deletions tests/manager/api/test_app.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
from unittest.mock import patch
from unittest.mock import MagicMock, patch

from palace.manager.api.app import initialize_application
import pytest

from palace.manager.api.app import (
initialize_application,
initialize_circulation_manager,
)
from palace.manager.util.http import HTTP
from tests.fixtures.database import DatabaseTransactionFixture
from tests.fixtures.services import ServicesFixture
Expand All @@ -18,3 +23,30 @@ def test_initialize_application_http(

# Make sure that the HTTP configuration was set
mock_set_quick_failure_settings.assert_called_once()


def test_initialize_circulation_manager(caplog: pytest.LogCaptureFixture):
with (
patch("palace.manager.api.app.app") as mock_app,
patch("palace.manager.api.app.CirculationManager") as mock_circulation_manager,
patch("palace.manager.api.app.CachedData") as mock_cached_data,
):
# If app is already initialized, it should not be re-initialized
mock_app.manager = MagicMock()
initialize_circulation_manager()
mock_circulation_manager.assert_not_called()
mock_cached_data.initialize.assert_not_called()

# If app is not initialized, it should be initialized
mock_app.manager = None
initialize_circulation_manager()
mock_circulation_manager.assert_called_once()
mock_cached_data.initialize.assert_called_once()
assert mock_app.manager == mock_circulation_manager.return_value

# If an exception is raised, it should be logged
mock_app.manager = None
mock_circulation_manager.side_effect = Exception("Test exception")
with pytest.raises(Exception):
initialize_circulation_manager()
assert "Error instantiating circulation manager!" in caplog.text
7 changes: 1 addition & 6 deletions tests/manager/api/test_authenticator.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import datetime
import json
import os
import re
from collections.abc import Callable
from decimal import Decimal
Expand Down Expand Up @@ -1020,12 +1019,8 @@ def _geographic_areas(cls, library):

# We're about to call url_for, so we must create an
# application context.
os.environ["AUTOINITIALIZE"] = "False"
from palace.manager.api.app import app

self.app = app
del os.environ["AUTOINITIALIZE"]

# Set up configuration settings for links.
library_settings.terms_of_service = "http://terms.com" # type: ignore[assignment]
library_settings.privacy_policy = "http://privacy.com" # type: ignore[assignment]
Expand Down Expand Up @@ -1091,7 +1086,7 @@ def _geographic_areas(cls, library):
finish=announcement_fixture.yesterday,
)

with self.app.test_request_context("/"):
with app.test_request_context("/"):
url = authenticator.authentication_document_url()
assert url.endswith("/%s/authentication_document" % library.short_name)

Expand Down