Support pluggable storage providers about nimbella-sdk-python

[joshuaauerbachwatson]

This looks good to me too. Bear in mind that I'm not into python and don't really know the eco-system. Some comments.

• I originally thought the set of storage providers would pluggable without any code modifications; this is why I use npm package names as the key. But, currently you have to modify the package.json dependencies to include new ones: that qualifies as code, by some definitions. And, in the workbench I am currently statically enumerating which ones can be loaded in order to get correct webpack behavior. And, I don't really believe we are going to have a lot of these. So, definitely don't kill yourself to make the extension mechanism fully dynamic. Do what works, even if it involves statically enumerating somewhere. If it helps, we can change the nature of the key.
• the fact that I source all three of the storage-provider related nodejs packages in the same repo with the cli and deployer was expedient ... don't feel there is any need to do the same for python and we might want to change it for nodejs. I think at some point we might want to rethink the overall structure after we are supporting a non-trivial number of languages (ie more than two 🙂 ).
• I'm all for type checking but I am also aware that all of the types I'm using now add up to probably a fraction of what we'll really need once customers start beating on us (if we're so lucky). Also, the error return and exception values from the current functions are untyped and also unabstracted: we do a good job on the input side, and for correct outputs, but when there are errors we throw the user on the mercy of the underlying implementation. So, for all of these reasons I'd be ok with postponing the use of static typing for python until we've gone a little further down the road of getting the abstractions right and complete.

[jthomas]

I've now finished implementating the individual AWS S3 storage client methods with unit tests which all pass. I'm now running the integration tests against this new storage provider and fixing the bugs in the client. Once this is done - it'll be ready.

[jthomas]

Here are some design notes for others to review...

existing node.js implementation

Storage provider implementation is configured using an optional property (provider) on the __NIM_STORAGE_KEY environment variable. This defaults to GCP if missing. Provider identifers are the NPM package names for storage implementations, e.g. @nimbella/storage-gcs.

In the makeStorageClient method, the storage plugin is dynamically loaded at runtime. The storage credentials are passed to the prepareCredentials method on the storage implementation and this result is passed to the getClient method to return an implementation of the storage client interface.

Implementations for the GCP and AWS object stores are found here (and published as separate modules): https://github.com/nimbella/nimbella-cli

This module defines the abstract types for the StorageProvider, StorageClient, RemoteFile interfaces.

possible python implementation

We should follow the existing Node.js API interfaces for storage client abstractions as closely as possible - ensuring consistency across languages. This looks pretty straight forward for the main classes (StorageProvider, StorageClient, etc...). It probably doesn't make sense for the smaller pure data types (e.g. GetFileOptions) due to Python's dynamic typing nature.

• It would be nice to introduce some static typing support into the library using Python's new type annotations feature. We can run a type-linting system during the build process to help us catch errors before publishing. Python Data classes and type annotations can be used for some of the smaller classes to replicate the interfaces from TS.

• Dynamic module loading is handled in Python by the importlib package. We need to translate from the storage provider identifier (an external NPM module name) to the local Python storage package. I don't want to over-complicate storage module loading so will not attempt to split out the storage implementations into separate PyPi packages (unlike the Node.js version). There's a nice example here Python decorators to implement a plugin system for modules. I'd recommend using this approach to allow us to dynamically register storage provider plugins.

• Testing. I'll create unit tests for the storage client and providers using Python's mock library to run locally during development. I'll also create a series of integration tests that can be run against real storage provider backends. We can have system tests in the main Jenkins build by creating actions using these libraries (following the approach of the Node.js library).

What is the minimum version of Python we need to support with the SDK? @rabbah

[rabbah]

The overall direction lgtm. I defer to Josh for more detailed critique since he implemented the Node version. For python version, min version should be 3.7.

[jthomas]

Thanks for the detailed feedback @joshuaauerbachwatson, this is really helpful. - I'll make a start on this next. I won't let "perfect be the enemy of good" with regardings to the plugin system & type checking and just get something simple working to begin with.

[jthomas]

Progress update: I've finished the plugin manager class, the abstract base class for storage plugins and the GCS storage plugin. I need to work on the AWS S3 plugin next and then write some simple integration tests to actually check they work against example services. Details in #2.

I'm using Python typings to support run a static type checker during the build step to catch errors. I'm also running the unit tests using Github Actions as well.

[jthomas]

I've now integrated the dynamic plugin loading into the SDK storage() function. I've written an integration test that uses the client API to interact with the storage service to verify it works. This integration test only uses external storage interface and can be ran with any of the provider plugins (environment credentials used to pass secrets and determine which provider is used).

I've got a few small todos to finish but can now start on the AWS S3 support. This should be straightforward as I just need to implement a second provider plugin (using the first as a template). See details in #2.

[jthomas]

I've made good progress on the AWS S3 provider plugin. I've got 60% of the methods implemented and unit-tested. Will finish these implementations off next week and check it works with the integration tests. Updates in #2.

[jthomas]

#2 is now ready for review. The provider-independent integration tests work against the AWS S3 backend 👏. The AWS S3 classes are finished & unit-tested. I've opened a few small issues (#3, #4) to resolve before we can call this complete but it's ready as a v1. Once this is merged, we can look at how to release a new version.

[jthomas]

This is now finished and ready for publishing a new major version of the library.