move inf model IsCritial func out of datastore

[nirrozenbaum]

InferenceModel IsCritical function doesn't belong to Datastore.
today we have in code only in-memory datastore. if tomorrow we'll add Redis based datastore (or any other cache) we would still need this helper function.

Datastore responsibility is to hold inf pool, models and pod metrics.

[netlify]

✅ Deploy Preview for gateway-api-inference-extension ready!

Name	Link
🔨 Latest commit	ca5f939
🔍 Latest deploy log	https://app.netlify.com/sites/gateway-api-inference-extension/deploys/67f6a0f2bc22360008c344c5
😎 Deploy Preview	https://deploy-preview-670--gateway-api-inference-extension.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[kfswain]

I'm not in love with this package name, and I wonder if we should create a dedicated package when we only have 1 func that uses it...

That being said I understand why we are suggesting this. Im torn, I'm wondering if this is a data access concern? It's essentially syntactic sugar for the criticality field read, and we probably need to articulate our criticality beyond just IsCritical (but that is out of scope of this PR for sure, just kinda thinking aloud).

[kfswain]

Overall this LGTM, I do have some concerns about the util package pattern we are setting. I could see this becoming a junk-drawer directory, but I'll let others weigh in.

Regardless, thank you for taking the time and effort to refactor our codebase and clean it up.

cc: @ahg-g @danehans

[nirrozenbaum]

@kfswain I'm also fine with putting this function in other place. it's also possible to get rid of it completely and do this one line check directly in the two places it's being used.

as long as it gets out of datastore 🙂

[ahg-g]

Overall this LGTM, I do have some concerns about the util package pattern we are setting. I could see this becoming a junk-drawer directory, but I'll let others weigh in.

Regardless, thank you for taking the time and effort to refactor our codebase and clean it up.

cc: @ahg-g @danehans

under util we have specific pkgs, so there is some form of organization already,,, but I am aligned that IsCritical is not something I expect to see in the datastore pkg.

[kfswain]

it's also possible to get rid of it completely and do this one line check directly in the two places it's being used.

Yeah, this SGTM. I'm gonna be doing some refactoring over the next couple days so this should actually go down to 1 place anyway.

[nirrozenbaum]

I'm also in the middle of refactoring datastore and the backend/metrics... in other WIP pr.. will get rid of it for now

[kfswain]

/lgtm
/approve

This is great! Ty

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kfswain, nirrozenbaum

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [kfswain]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment