dataorc: tiny wrappers issue

[tokams]

dataorc/packages/dataorc-utils/src/dataorc_utils/azure/keyvault.py

Line 38 in 5e6e31a

cred: Any = DefaultAzureCredential()

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:

dataorc/packages/dataorc-utils/src/dataorc_utils/azure/keyvault.py

Line 3 in 5e6e31a

The helpers use the Azure SDK when available and fall back to environment

[tokams]

My concern here is that we can end up in two situations:

• Everyone contributes with ther own little helper methods that dont bring any real functionality but make the code look cleaner, obfuscating logic. At some point we might loose track of if and end up having multiple utilities doing the same thing in the same way or slightly different way.

• Constraints for new features are too tight making it too hard to contribute and we end up developing our own utilities and never sharing the code.

I think we need to set some proper constraints, and have regular checkups on what each team is developing for their purposes in terms of utilities, and concider those as potential dataorc-utils.

[tokams]

#55

[hknutsen]

@tokams I'd say this is somewhat related to #56.