• 
      

    Reduce startup times for rbssh and make custom SSH backends persistant.

    Review Request #11002 — Created April 13, 2020 and submitted

    Information

    Review Board
    release-3.0.x

    Reviewers

    rbssh has historically executed within a Review Board environment in
    order to take advantage of extension support, necessary in order for the
    flexible SSH key storage backends to take effect. Originally, startup
    time was fairly reasonable, but over time this became more expensive,
    and on large deployments (particularly those using many extensions, or
    expensive-to-load extensions) it could be a bottleneck.

    This change reworks execution by running in a minimal Django
    environment. It now uses a custom settings.py, which loads only the
    bare-minimum Django apps needed to manage and implement SSH key storage
    backends.

    This means that extensions are no longer loaded. In order for an
    extension-supplied storage backend to still work, the supplier of that
    backend must set the siteconfig-provided ssh_storage_backend to the
    path of the backend class. This supercedes the now-legacy
    RBSSH_STORAGE_BACKEND key in settings.py. It also means that
    extensions must be updated to set the new key in order to continue
    working. Unlike the old process-local setting, the new setting
    persists in the database, allowing rbssh to look it up without loading
    any extensions.

    Backend loading will try the new setting and the old setting, which will
    help when performing SSH-backed requests from within the Review Board
    process, or in rbssh if settings_local.py defines
    RBSSH_STORAGE_BACKEND.

    SSH communication within rbssh has also received some small speed
    tweaks. We no longer attempt to log anything to debug if the
    $DEBUG_RBSSH environment variable isn't set. We previously would send
    to the logger, which would go through the internal logging processes and
    then get ignored. We now just don't do anything.

    NOTE: This is a breaking change:

    1. Extension developers providing custom SSH key storage backends will
      need to ensure their backends run within a bare-minimum environment.
      They can rely on the Django Site and Djblets SiteConfiguration
      models, but nothing else. If this impacts anyone, they will need to
      talk to us about a future-proof solution, but this change is not
      currently attempting to over-engineer this, since we're only aware of
      the custom backend provided by Power Pack.

    2. Power Pack users will need to update to Power Pack 3.0.3 (still
      in-development at the time of this change) in order to continue using
      DB-backend SSH keys, or will need to set the following in
      settings_local.py:

    RBSSH_STORAGE_BACKEND = 'rbpowerpack.sshdb.storage.DBSSHStorage'
    

    Unit tests pass.

    Manually invoked rbssh, running speed tests and comparing the old
    execution time to the new one. Saw a drastic speed reduction, particularly
    with extensions enabled.

    Tested usage from within Review Board.

    Tested with the default local keys backend and the Power Pack SSHDB backend
    (using an updated Power Pack with support for the new setting), verifying
    that the INSTALLED_APPS changes didn't break anything.

    Tested these changes on both Review Board 3.0 and 4.0.

    Summary ID
    Reduce startup times for rbssh and make custom SSH backends persistant.
    rbssh has historically executed within a Review Board environment in order to take advantage of extension support, necessary in order for the flexible SSH key storage backends to take effect. Originally, startup time was fairly reasonable, but over time this became more expensive, and on large deployments (particularly those using many extensions, or expensive-to-load extensions) it could be a bottleneck. This change reworks execution by running in a minimal Django environment. It now uses a custom `settings.py`, which loads only the bare-minimum Django apps needed to manage and implement SSH key storage backends. This means that extensions are no longer loaded. In order for an extension-supplied storage backend to still work, the supplier of that backend must set the siteconfig-provided `ssh_storage_backend` to the path of the backend class. This supercedes the now-legacy `RBSSH_STORAGE_BACKEND` key in `settings.py`. It also means that extensions *must* be updated to set the new key in order to continue working. Unlike the old process-local setting, the new setting persists in the database, allowing rbssh to look it up without loading any extensions. Backend loading will try the new setting and the old setting, which will help when performing SSH-backed requests from within the Review Board process, or in rbssh if `settings_local.py` defines `RBSSH_STORAGE_BACKEND`. SSH communication within rbssh has also received some small speed tweaks. We no longer attempt to log anything to debug if the `$DEBUG_RBSSH` environment variable isn't set. We previously would send to the logger, which would go through the internal logging processes and then get ignored. We now just don't do anything. **NOTE:** This is a breaking change: 1. Power Pack users will need to update to Power Pack 3.0.3 (still in-development at the time of this change) in order to continue using DB-backend SSH keys, or will need to set the following in `settings_loca.py`: ```python RBSSH_STORAGE_BACKEND = 'rbpowerpack.sshdb.storage.DBSSHStorage ``` 2. Extension developers providing custom SSH key storage backends will need to ensure their backends run within a bare-minimum environment. They can rely on the Django `Site` and Djblets `SiteConfiguration` models, but nothing else. If this impacts anyone, they will need to talk to us about a future-proof solution, but this change is not currently attempting to over-engineer this, since we're only aware of the custom backend provided by Power Pack.
    14654f9109c307a6f02b66dd4f2feaf55de22026
    Description From Last Updated

    F403 'from reviewboard.settings import *' used; unable to detect undefined names

    reviewbot reviewbot

    F401 'reviewboard.settings.*' imported but unused

    reviewbot reviewbot

    E402 module level import not at top of file

    reviewbot reviewbot

    E402 module level import not at top of file

    reviewbot reviewbot

    E731 do not assign a lambda expression, use a def

    reviewbot reviewbot
    Checks run (1 failed, 1 succeeded)
    flake8 failed.
    JSHint passed.

    flake8

    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-3.0.x (aa2a76b)