equinor / equinor/dataorc

dataorc: tiny wrappers issue

Open
#54 3 comments 2 reactions 1 assignee Claimed by @tokams View on GitHub
enhancement
Dominant language
Python
Stars
1
Forks
0
PR merge metrics
No merged PRs in 30d

Description

https://github.com/equinor/dataorc/blob/5e6e31af1c27313b3241c4b0c270d2723ff82173/packages/dataorc-utils/src/dataorc_utils/azure/keyvault.py#L38

If i want to fetch 10 secrets for my configuration, using this utility function with dataorc-utils will create a new instance of `DefaultAzureCredential` and `AzureAppConfigurationClient`.

Not sure how helpful is using this utility instead of manually creating one instance

```
credential = DefaultAzureCredential()

ac_client = AzureAppConfigurationClient(
base_url=e("APP_CONFIG_ENDPOINT"),
credential=credential,
)

```

Especially if we will very likely also require credentials instance in our code anyway for other purposes not covered in dataorc-utils
This function essencialy wraps class initialization, and exposes output from one of their methods.

Utilities or wrapper functions obfuscate code, especially if they do not bring any meaninful functionality.

I think we should have stronger constraints on what can be a dataorc-utils utility function.

For this case we should add at least some meaningfull funcitonality, either client factory, so those can be reused, or dependency injection whre the helper will accept overrides for credentials/kv.

Also this promis is not implemented:
https://github.com/equinor/dataorc/blob/5e6e31af1c27313b3241c4b0c270d2723ff82173/packages/dataorc-utils/src/dataorc_utils/azure/keyvault.py#L3

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.