console-shared: Move pod's getNodeName() selector

[mareklibra]

Will be reused by cluster dashboard.

Depends on:

• kubevirt: Add VM dashboard #2600

Just the last commit is relevant.

[spadgett]

/hold

These selectors are an anti-pattern imo. We should be trying to remove them instead of adding more.

[spadgett]

/hold cancel

[mareklibra]

These selectors are an anti-pattern imo. We should be trying to remove them instead of adding more.

@spadgett , Can you please explain? I see following benefits when used:

• shared "parsing" logic for input
• better maintenance as the "logic coded as a string" is at one place only
• better use of typescript
• better descriptive (by function name)
• possibility to track its use using IDE
• single place to be changed if API changes (ideally)

[mareklibra]

Sorry, I missed your comment in #2808

[spadgett]

• shared "parsing" logic for input

This isn't parsing anything. It's returning a straight property value.

• better maintenance as the "logic coded as a string" is at one place only

We shouldn't be doing "logic coded as a string" at all. That's my point :)

• better use of typescript

_.get is not type safe.

• better descriptive (by function name)

IMO, it actually obscures what's really happening for someone who knows the API. You don't know if there's extra logic in the helper or what field it's really returning. It's an unnecessary level of indirection.

• possibility to track its use using IDE

Shouldn't this still be possible checking references to a property in a type definition? In fact, I think this makes it harder to know where things are being used.

• single place to be changed if API changes (ideally)

These APIs are stable. I see more of an argument for these helpers for alpha APIs, but that's not how we've been using these.

[mareklibra]

shared "parsing" logic for input

This isn't parsing anything. It's returning a straight property value.

Right, that's the reason for the quotes. But as there is no validation when parsing (or following the parser), the selectors can be the place to safely and once deal with unexpected input.

better maintenance as the "logic coded as a string" is at one place only

We shouldn't be doing "logic coded as a string" at all. That's my point :)

I agree. I have changed this PR to avoid _.get(). I still think the selectors can simplify the code for deeper paths.

[mareklibra]

Rebased.
@spadgett , can you please have a look again?

[spadgett]

/approve
/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mareklibra, spadgett

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• frontend/OWNERS [spadgett]

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