search

singularityhub / sregistry-cli

Singularity Global Client for container management
https://singularityhub.github.io/sregistry-cli/
Mozilla Public License 2.0
14 stars 18 forks source link

SREGISTRY_DATABASE directory must be created by user #216

Closed tschoonj closed 5 years ago

tschoonj commented 5 years ago

Describe the bug

When setting SREGISTRY_DATABASE, to a non-existent location, sregistry images exits with an exception:

Traceback (most recent call last):
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 2262, in _wrap_pool_connect
    return fn()
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 354, in connect
    return _ConnectionFairy._checkout(self)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 751, in _checkout
    fairy = _ConnectionRecord.checkout(pool)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 483, in checkout
    rec = pool._do_get()
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/pool/impl.py", line 237, in _do_get
    return self._create_connection()
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 299, in _create_connection
    return _ConnectionRecord(self)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 428, in __init__
    self.__connect(first_connect_check=True)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 630, in __connect
    connection = pool._invoke_creator(self)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/engine/strategies.py", line 114, in connect
    return dialect.connect(*cargs, **cparams)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/engine/default.py", line 453, in connect
    return self.dbapi.connect(*cargs, **cparams)
sqlite3.OperationalError: unable to open database file

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/bin/sregistry", line 11, in <module>
    load_entry_point('sregistry', 'console_scripts', 'sregistry')()
  File "/home/awf63395/github/sregistry-cli/sregistry/client/__init__.py", line 333, in main
    subparser=subparsers[args.command])
  File "/home/awf63395/github/sregistry-cli/sregistry/client/images.py", line 19, in main
    cli = get_client(quiet=args.quiet)
  File "/home/awf63395/github/sregistry-cli/sregistry/main/__init__.py", line 111, in get_client
    cli = Client()
  File "/home/awf63395/github/sregistry-cli/sregistry/main/s3/__init__.py", line 35, in __init__
    super(Client, self).__init__(**kwargs)
  File "/home/awf63395/github/sregistry-cli/sregistry/main/base/__init__.py", line 59, in __init__
    self._init_db(SREGISTRY_DATABASE)
  File "/home/awf63395/github/sregistry-cli/sregistry/database/models.py", line 158, in init_db
    Base.metadata.create_all(bind=self.engine)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/sql/schema.py", line 4287, in create_all
    ddl.SchemaGenerator, self, checkfirst=checkfirst, tables=tables
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 2032, in _run_visitor
    with self._optional_conn_ctx_manager(connection) as conn:
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/contextlib.py", line 112, in __enter__
    return next(self.gen)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 2024, in _optional_conn_ctx_manager
    with self._contextual_connect() as conn:
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 2226, in _contextual_connect
    self._wrap_pool_connect(self.pool.connect, None),
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 2266, in _wrap_pool_connect
    e, dialect, self
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1536, in _handle_dbapi_exception_noconnection
    util.raise_from_cause(sqlalchemy_exception, exc_info)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/util/compat.py", line 383, in raise_from_cause
    reraise(type(exception), exception, tb=exc_tb, cause=cause)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/util/compat.py", line 128, in reraise
    raise value.with_traceback(tb)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 2262, in _wrap_pool_connect
    return fn()
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 354, in connect
    return _ConnectionFairy._checkout(self)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 751, in _checkout
    fairy = _ConnectionRecord.checkout(pool)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 483, in checkout
    rec = pool._do_get()
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/pool/impl.py", line 237, in _do_get
    return self._create_connection()
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 299, in _create_connection
    return _ConnectionRecord(self)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 428, in __init__
    self.__connect(first_connect_check=True)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 630, in __connect
    connection = pool._invoke_creator(self)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/engine/strategies.py", line 114, in connect
    return dialect.connect(*cargs, **cparams)
  File "/dls_sw/apps/python/anaconda/4.6.14/64/envs/python3.7/lib/python3.7/site-packages/sqlalchemy/engine/default.py", line 453, in connect
    return self.dbapi.connect(*cargs, **cparams)
sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) unable to open database file
(Background on this error at: http://sqlalche.me/e/e3q8)

This problem can be solved easily by running mkdir -p $SREGISTRY_DATABASE before executing sregistry images

To Reproduce Steps to reproduce the behavior:

Set SREGISTRY_DATABASE to a non-existent directory and execute sregistry images.

Expected behavior

IMHO sregistry should try creating the folder itself (perhaps with os.makedirs) before trying to create the new sqlite database file. Only if this fails, an error should be returned.

Version of Singularity and Singularity Registry Client

> sregistry version
0.2.16
vsoch commented 5 years ago

Hmm, I disagree here - I think the application shouldn't be allowed to do a mkdir p, is it not reasonable to ask the user to select a folder that exists?

vsoch commented 5 years ago

What we should do, however, is do a check and exit gracefully instead of spitting all that out. Let me know your thoughts.

tschoonj commented 5 years ago

I agree that at the very least a graceful exit should happen.

Does this also occur when SREGISTRY_DATABASE is not set: is $HOME/.sregistry automatically created?

vsoch commented 5 years ago

If it's not set, the credentials file defaults to $HOME/.sregistry for credentials, and $HOME/.singularity/sregistry.db for the sqlite database. Those should be taken care of for creation. You can see the flow of logic in https://github.com/singularityhub/sregistry-cli/blob/master/sregistry/defaults.py#L103 and then init_db is called here and the function is here (so that's where you'd want to check for the path and gracefully exit). And then interaction with the credentials file is here and clients that require secrets can call self.require_secrets() (in auth.py in that folder). In terms of reading /loading, that's done here and it looks like we do a mkdir-p since it's just a file.

tschoonj commented 5 years ago

Ok, I see what happens now. Will write a patch for the graceful exit.

Thanks for the help!

vsoch commented 5 years ago

That's what I'm here for! :)