Adding SecretFetcher to pyproject.toml
#40
Conversation
| @@ -55,6 +55,7 @@ metaflow = ["zillow-metaflow"] | |||
| dask = ["dask"] | |||
| spark = ["pyspark"] | |||
| kubernetes = ["kubernetes"] | |||
| SecretFecther = ["SecretFecther"] | |||
There was a problem hiding this comment.
Curious what's the intended usage with this change? I was under the assumption that the right portion of the equation here needs to be some python libraries that this package depends on (like pyspark, kubernetes, etc. above), but here SecretFecther is something we define ourselves but not a package dependency?
There was a problem hiding this comment.
The goal is to reorganize the dependencies such if the user only want to access secrets that they don't need to also pip install the s3 libraries [s3fs, arrow, parquet, etc].
maybe the extra should be called boto3 instead of SecretFetcher because we are using aws? The extra name of boto3 communicates which dependency we are getting while SecretFetcher is the SDK abstraction. Or the extra be named aws and kubernetes for k8s secrets (which already exists).
There was a problem hiding this comment.
That makes sense. I included SecretFetcher as mentioned in the ticket for on-call. @talebzeghmi Just boto3 would be enough though?
There was a problem hiding this comment.
Plus kubernetes maybe? Might need some testing.
We'd also have to move out the s3 libraries into another s3 extra, which makes this a breaking change forcing a schema major version bump...
There was a problem hiding this comment.
Yeah mostly I'm taking this extra pattern as a way to separate out components and dependencies we do not always need. However, in this current pattern if we always need secret fetcher, we do not need to put it as extra? Basically wondering what actual library is blocking us.
There was a problem hiding this comment.
What was preventing Yanik from using zdatasets is that he didn't want the set of s3 libraries [s3fs, arrow, parquet, etc] installed if he is only using SecretFetcher.
Could there be a default extra, say named s3?
There was a problem hiding this comment.
Yeah that could make more sense!
pyproject.tomlpoetry.lock