The project is not the problem -- it's the name used for the inference service and/or servingruntime, no?

Updated

Curious why you didn't just update the kserveMetricsSupported to include "not NIM"?

Although this code block we have looks very weird lol

I did in the beginning but then I saw that modelMeshMetricsSupported and kserveMetricsSupported had duplicated logic, so I chose to add the isKServeNIMEnabled check separately for clarity and to make the logic explicit. I can update the kserveMetricsSupported to handle the "not NIM" case if you think it would make it cleaner.

Yeah, I noticed that -- the more you change the longer it takes for me to verify and approve. I'll try to get this before my PTO -- but typically stay short and to the point unless the logic is not compatible. Every cleanup you do for us has me testing more to make sure we don't have some other brittle aspect that can slip out in non NIM ways. In the end it's likely a net positive, just expressing how I look at everything.

Hi @olavtar pardon me cause I don't have all the context, but shouldn't this produce the same result?

        <ResourceNameTooltip resource={inferenceService}>
          {modelMeshMetricsSupported || kserveMetricsSupported ? (
            <Link
              data-testid={`metrics-link-${displayName}`}
              to={
                isGlobal
                  ? `/modelServing/${inferenceService.metadata.namespace}/metrics/${inferenceService.metadata.name}`
                  : `/projects/${inferenceService.metadata.namespace}/metrics/model/${inferenceService.metadata.name}`
              }
            >
              {displayName}
            </Link>
          ) : (
            displayName
          )}
        </ResourceNameTooltip>

There's value on refactoring duplicate code, but how NIM is going to affect this component if the fallback is the same as having the feature selected? Maybe for readability, but in case we wanna add later metrics, we can add a comment explaining that nim will be enabled when we support metrics.

Hi @lucferbux, the isKServeNIMEnabled check is important because we don’t want to show a link when NIM Metrics isn't enabled, even if other metrics conditions are met. So, removing this condition would change the behavior, as it would allow the link to be displayed when NIM Metrics are not enabled, which is not the intended outcome.