Open thomasborgen opened 2 years ago
@thomasborgen I quickly looked through the implementation. I cannot formulate now what I would change but the implementation looks similar to what we discussed. I need to sleep on it.
Note: Maybe it is because of typing but I got the first impression that it is slightly more complicated than necessary. Maybe it is only an impression.
@thomasborgen I quickly looked through the implementation. I cannot formulate now what I would change but the implementation looks similar to what we discussed. I need to sleep on it.
Note: Maybe it is because of typing but I got the first impression that it is slightly more complicated than necessary. Maybe it is only an impression.
@ChameleonTartu
Yes, the typing with the overloading is quite annoying yes. But we should keep typing and some overhead in this package, is worth it when getting correct type warnings for users.
One way to make it a bit easier is to use singledispatch
from stdlib or classes
to let the type of the argument decide what function to call. so its like this
params = GCPCreateBucket(...)
create_bucket(params)
Still very much a WIP but the idea is that we register functions with decorators, and get them when needed.
So in package
storage_bucket.aws.create_bucket
we have functionaws_create_bucket()
with is decorated with@register_create_bucket(platform=Platform.aws)
And then in root packagestorage_bucket.create_bucket
we have the api functioncreate_bucket()
which takes aenum Platform
, and theget_create_bucket(platform)
function instorage_bucket.registries
retrieves the registered function for the correct platform.This could work quite nicely.
Another nice thing about this approach is that the code is nice and separated. This means that the aws_create_bucket function can call its own aws_get_client() without us having to for example resolve it earlier in the pipe and pass it to the function.
Right now I've tried with
BaseCreateBucket
input dataclass that the platform spesific functions can subclass, but maybe some other approach is better.The main.py file is there for testing purposes only and can be run with
poetry run python storage_bucket
@ChameleonTartu got any thoughts on this?
I can say that I tried having CloudClient and have that subclassed by each platform based Client, but the abstract methods would not work with any other input that the CloudClient had demanded. So I couldn't like have a
BaseInput
andSpecificAWSInput(BaseInput)
as type parameters. :( Might have just been mypy issue but after a bit of reading it seems like it wasn't the best of ideas.