Comment out _find_image_pull_secrets_values_paths().#168
Comment out _find_image_pull_secrets_values_paths().#168
Conversation
|
Do not merge until webhooks story is done (not the current one, new one that will be raised and link here). We do need this, spoke to Jacob |
| @@ -242,9 +242,12 @@ def _generate_artifact_profile(self) -> AzureArcKubernetesArtifactProfile: | |||
| """ | |||
| logger.debug("Generating artifact profile for Helm chart input.") | |||
| image_pull_secrets_values_paths: Set[str] = set() | |||
There was a problem hiding this comment.
This is what the function updates according to Jacob. So we cannot remove this code
There was a problem hiding this comment.
Thanks for checking Jordan. Can we make this clearer in the code? Perhaps either rename the matches argument image_pull_secrets_values_paths, or doc it much more clearly?
There was a problem hiding this comment.
I had a thought on how to refactor this so we have a clear return value rather than the hard to read modification of a passed by reference variable.
Wrap the current recursive method in an outer method that sets up the initial variable, and returns a clear final value for image_pull_secrets_values_paths.
Expect it can wait till I'm back.
2 participants
I cannot see that this method actually does anything. I commented it out and haven't seen any problems in the various tests I've run, though these haven't been comprehensive.
For now, I'm just leaving as commented out, so that if it turns out it is needed, it's easier to spot what's gone wrong, and fix.
If we've still not hit problems after some soak time, we should delete the code.