LibreTexts / ngshare

nbgrader sharing service
https://ngshare.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
17 stars 17 forks source link

Another way to remove creation of temporary files for alembic #73

Closed lxylxy123456 closed 4 years ago

lxylxy123456 commented 4 years ago

Use temporary files and access using fd. Avoid going into alembic's internal APIs. Closes #70

rkevin-arch commented 4 years ago

I still think we should go ahead and use #70. These internal APIs are intended to be used (quote: However, real-world applications will usually end up using more of the internal API, in particular being able to run commands programmatically, as discussed in the section Commands.).

The current fix still crashes (because the filesystem doesn't like tempfile.TemporaryFile()):

Traceback (most recent call last):
  File "/ngshare/ngshare.py", line 863, in <module>
    main()
  File "/ngshare/ngshare.py", line 848, in main
    dbutil.upgrade(args.database)
  File "/ngshare/dbutil.py", line 78, in upgrade
    with _temp_alembic_ini(db_url) as (alembic_ini, fd):
  File "/usr/lib/python3.6/contextlib.py", line 81, in __enter__
    return next(self.gen)
  File "/ngshare/dbutil.py", line 60, in _temp_alembic_ini
    tf = tempfile.TemporaryFile()
  File "/usr/lib/python3.6/tempfile.py", line 729, in TemporaryFile
    prefix, suffix, dir, output_type = _sanitize_params(prefix, suffix, dir)
  File "/usr/lib/python3.6/tempfile.py", line 269, in _sanitize_params
    dir = gettempdir()
  File "/usr/lib/python3.6/tempfile.py", line 437, in gettempdir
    tempdir = _get_default_tempdir()
  File "/usr/lib/python3.6/tempfile.py", line 372, in _get_default_tempdir
    dirlist)
FileNotFoundError: [Errno 2] No usable temporary directory found in ['/tmp', '/var/tmp', '/usr/tmp', '/srv/jupyterhub']

I changed the filesystem to be not read-only, and this has fixed the issue, but other problems like the import at https://github.com/lxylxy123456/ngshare/blob/6ef2fd3b0ee8bd64d28e70e5508152cd21201b0b/ngshare/alembic/env.py#L18 is still causing issues. Even after I fix all of that, the alembic call takes around 0.25s instead of 0.013s for the alembic_ini_no_temp branch when there is no update. I know this isn't much since it's not called frequently, but I just don't see the point of dealing with temporary files and context managers when we can just use the alembic API.

lxylxy123456 commented 4 years ago

Use of fd is not desired.