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

[API change] Getting rid of the create method for observers v0.99999 #601

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion docs/observers.rst
Expand Up @@ -583,7 +583,11 @@ To add a SqlObserver from python code do:

from sacred.observers import SqlObserver

ex.observers.append(SqlObserver.create('sqlite:///foo.db'))
ex.observers.append(SqlObserver('sqlite:///foo.db'))

# It's also possible to instantiate a SqlObserver with an existing
# engine and session with:
ex.observers.append(SqlObserver.create_from(my_engine, my_session))


Schema
Expand Down
21 changes: 19 additions & 2 deletions sacred/observers/sql.py
Expand Up @@ -3,6 +3,7 @@

import json
from threading import Lock
import warnings

from sacred.commandline_options import CommandLineOption
from sacred.observers.base import RunObserver
Expand All @@ -17,21 +18,37 @@
class SqlObserver(RunObserver):
@classmethod
def create(cls, url, echo=False, priority=DEFAULT_SQL_PRIORITY):
warnings.warn(
"SqlObserver.create(...) is deprecated. Please use"
" SqlObserver(...) instead.",
DeprecationWarning,
)
return cls(url, echo, priority)

def __init__(self, url, echo=False, priority=DEFAULT_SQL_PRIORITY):
from sqlalchemy.orm import sessionmaker, scoped_session
import sqlalchemy as sa

engine = sa.create_engine(url, echo=echo)
session_factory = sessionmaker(bind=engine)
# make session thread-local to avoid problems with sqlite (see #275)
session = scoped_session(session_factory)
return cls(engine, session, priority)
self.engine = engine
self.session = session
self.priority = priority
self.run = None
self.lock = Lock()

def __init__(self, engine, session, priority=DEFAULT_SQL_PRIORITY):
@classmethod
def create_from(cls, engine, session, priority=DEFAULT_SQL_PRIORITY):
"""Instantiate a SqlObserver with an existing engine and session"""
self = cls.__new__(cls) # skip __init__ call
self.engine = engine
self.session = session
self.priority = priority
self.run = None
self.lock = Lock()
return self

def started_event(
self, ex_info, command, host_info, start_time, config, meta_info, _id
Expand Down
8 changes: 4 additions & 4 deletions tests/test_observers/test_sql_observer.py
Expand Up @@ -48,7 +48,7 @@ def session(engine):

@pytest.fixture
def sql_obs(session, engine):
return SqlObserver(engine, session)
return SqlObserver.create_from(engine, session)


@pytest.fixture
Expand Down Expand Up @@ -228,7 +228,7 @@ def test_fs_observer_resource_event(sql_obs, sample_run, session, tmpfile):


def test_fs_observer_doesnt_duplicate_sources(sql_obs, sample_run, session, tmpfile):
sql_obs2 = SqlObserver(sql_obs.engine, session)
sql_obs2 = SqlObserver.create_from(sql_obs.engine, session)
sample_run["_id"] = None
sample_run["ex_info"]["sources"] = [[tmpfile.name, tmpfile.md5sum]]

Expand All @@ -240,7 +240,7 @@ def test_fs_observer_doesnt_duplicate_sources(sql_obs, sample_run, session, tmpf


def test_fs_observer_doesnt_duplicate_resources(sql_obs, sample_run, session, tmpfile):
sql_obs2 = SqlObserver(sql_obs.engine, session)
sql_obs2 = SqlObserver.create_from(sql_obs.engine, session)
sample_run["_id"] = None
sample_run["ex_info"]["sources"] = [[tmpfile.name, tmpfile.md5sum]]

Expand All @@ -255,7 +255,7 @@ def test_fs_observer_doesnt_duplicate_resources(sql_obs, sample_run, session, tm


def test_sql_observer_equality(sql_obs, engine, session):
sql_obs2 = SqlObserver(engine, session)
sql_obs2 = SqlObserver.create_from(engine, session)
assert sql_obs == sql_obs2

assert not sql_obs != sql_obs2
Expand Down
2 changes: 1 addition & 1 deletion tests/test_observers/test_sql_observer_not_installed.py
Expand Up @@ -12,7 +12,7 @@ def ex():
@pytest.mark.skipif(has_sqlalchemy, reason="We are testing the import error.")
def test_importerror_sql(ex):
with pytest.raises(ImportError):
ex.observers.append(SqlObserver.create("some_uri"))
ex.observers.append(SqlObserver("some_uri"))

@ex.config
def cfg():
Expand Down