seung-lab
feat: move cave secrets from secrets to cave_secrets
This will allow specifying the storage secrets and the CAVE secret at the same time.
can we do this is a deprecation warning kind of way and accumulate the breaking changes for a future major version change?
so for now as a patch change, add a deprecation warning that you should pass secrets in this other keyword argument, but still accept them in this one for graphene, then move the cleaner code version to a major version branch.
Yea that's no problem to keep it backwards compatible. I only considered doing it as breaking because there appear to be no references to secrets in PCG. I'll add in the shim.
It looks like the only code within the seung-lab github that would eventually need to be updated is this line: https://github.com/seung-lab/CAVEclient/blob/42e38a4fb8e0e73d8a911ca9edea18840f9ca786/caveclient/infoservice.py#L387
Not sure about other orgs though.
That code was just added (partially to deal with a case like this) a day or two ago and is not in use in the world outside a few of my notebooks. Every time any code anywhere else is generating a cloudvolume — from both analysis scripts and services — it has to use the current secrets argument through cloudvolume explicitly. The number of users who would be affected would span not only services but a significant fraction of people doing analysis or guidance scripting as well. This is a very significant breaking change and shouldn't be made lightly, even though I understand the hypothetical problem it solves.
For example, you could make a new parsing system that lets you specify secrets explicitly based on prefix, e.g.:
cloudvolume.Cloudvolume(..., secrets={'graphene': cave_token, 'gs': google_secret})
that would resolve the ambiguity in the case you described but allow the current pattern to work while allowing the less verbose current nomenclature to persist during a period of deprecation.
Referencing issue https://github.com/seung-lab/cloud-volume/issues/453
That makes sense to me and the design would accommodate growth. My goal is to always present as (seemingly) simple an interface as possible, so the old way will hopefully never be deprecated since that would impose a cost on every user since the most common use case is surely accessing raw precomputed from all kinds of datasets.
I think you're probably right that there are a few people using secrets as a parameter, but github search seems find few examples across all public codebases. My feeling is they probably mostly hiding in one-off scripts and Jupyter notebooks.
https://github.com/search?l=Python&p=1&q=CloudVolume+secrets&type=Code
I haven’t thought through the implications of this change but just for completeness https://github.com/search?l=R&q=CloudVolume+secrets&type=Code.
Thought about this some more. I don't think Casey's suggestion will work because the secrets you input are already dicts. I think cave_secret (with backwards compatibility) is fine though perhaps a more generic name can be chosen to anticipate other use cases.