2i2c-org / binderhub-service

https://2i2c.org/binderhub-service/
BSD 3-Clause "New" or "Revised" License
8 stars 3 forks source link

Do not put name of release in resource names by default #57

Closed yuvipanda closed 2 months ago

yuvipanda commented 11 months ago

If this chart is installed on a namespace named 'imagebuildig-demo' with the name 'imagebuilding-demo', then the service object is called 'imagebuilding-demo/imagebuilding-demo-binderhub-service'. This has a couple of negatives:

  1. There is no single well known url that can be used to setup the service in hub config. It would have to look like this:
      services:
        binder:
          url: http://imagebuilding-demo-binderhub-service:8090

    And require copy pasting for every single deployment, and is error prone.

  2. When using kubectl, this really clutters the view and makes it quite difficult for me to see what is going on. I have to intentionally ignore more than half the characters in names before finding useful information. Reminds me of the dark days of https://en.wikipedia.org/wiki/Hungarian_notation.
  3. The benefit here is that multiple deployments can exist in the same namespace. However, this is an edge case, and not something that should be default optimized for.
  4. There's no way to actually get more concise names by twiddling with config. I tried different variants of the following to no effect:
    binderhub-service:
      fullnameOverride: ""
      nameOverride: ""

I know this is the default behavior of the directory that helm generates, but I'd appreciate us adopting what zero-to-jupyterhub does instead. It defaults to human readable names by default, and for the edge case of people trying to run multiple instances of the chart in one namespace, they can actually set override if they need be.

consideRatio commented 2 months ago

Review -> merge of https://github.com/2i2c-org/binderhub-service/pull/119 could close this.

yuvipanda commented 2 months ago

@consideRatio I'd recommend asking one of @sgibson91 or @GeorgianaElena for help reviewing this. Let's open a follow-up ticket about updating 2i2c's infrastructure to use the new setup.

Actually, I've changed my mind. In this instance, given this project is still under the 2i2c umbrella, I'd suggest going with https://infrastructure.2i2c.org/contributing/code-review/#prime-directive instead. Which is to ask - what do you feel uncomfortable about with respect to this PR that another individual person can help with? Can you test it to know it works? If so, I'd recommend testing it and then merging it that way. If there's anything missed, I think that would be caught when we have to change our infrastructure to match.

yuvipanda commented 2 months ago

I've opened https://github.com/2i2c-org/infrastructure/issues/4370 to track bringing the change in once this is completed.