docker-plugin icon indicating copy to clipboard operation
docker-plugin copied to clipboard

Improving uniqueness of templateID string (via template name)

Open cpfeiffer opened this issue 2 years ago • 3 comments

Just as in #816 and #874, we want to distinguish templates that use the same docker image.

We do this pragmatically by using the user-supplied template name, so that the user is in control to make it unique.

So this fixes #816 as long as the user provides distinguished template names, which is explained by the help texts.

Testing done

  • unit testcases were extended and pass
  • code has been used successfully in production for 10 months

Submitter checklist

  • [x] Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • [x] Ensure that the pull request title represents the desired changelog entry
  • [x] Please describe what you did
  • [x] Link to relevant issues in GitHub or Jira
  • [x] Link to relevant pull requests, esp. upstream and downstream changes
  • [ ] Ensure you have provided tests - that demonstrates feature works or fixes the issue

cpfeiffer avatar Jun 01 '23 10:06 cpfeiffer

Fair enough. We chose to be defensive here. So far, the template name has been completely optional and defaulted to just "docker". We didn't change that behavior, but gave the user an option to fix this issue. Suddenly asking this to be unique might upset a few of the 38k users of this plugin. I wouldn't mind, though.

cpfeiffer avatar Jun 01 '23 19:06 cpfeiffer

To avoid bothering people who do not need to be bothered, we could only issue a warning when (a) there is more than one template and (b) two templates resolve to the same effective value (e.g., two blank templates, one blank template and one template named "docker", two templates named "docker", two templates named "my-template", etc). The code depends on this for correctness, and you are warning people about it in the help, but I think the warning should be more prominent than that to avoid supportability challenges. I do not think this is a big ask since you are already pushing for this in the help text.

basil avatar Jun 01 '23 19:06 basil

FYI, I have removed myself from the list of maintainers for this plugin, so even if the requested changes are made and I approve this PR, I would still not have permissions to do a release. If this PR is important to you, I would recommend adopting this plugin.

basil avatar Jun 05 '23 19:06 basil