refactor(core): Update with_overrides signatures and type hints

[pingsutw]

Tracking issue

NA

Why are the changes needed?

Cannot know the args that with_override take

What changes were proposed in this pull request?

Update with_overrides signatures and type hints

How was this patch tested?

local

Setup process

NA

Screenshots

Check all the applicable boxes

• [x] I updated the documentation accordingly.
• [x] All new and existing tests passed.
• [x] All commits are signed-off.

Related PRs

NA

Docs link

NA

[pingsutw]

cc @fg91 since you're also using with_overrides

[fg91]

@fellhorn fyi

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 91.48936% with 4 lines in your changes missing coverage. Please review.

Project coverage is 71.62%. Comparing base (83b90fa) to head (c3af69c). Report is 2 commits behind head on master.

Files	Patch %	Lines
flytekit/core/node.py	90.24%	1 Missing and 3 partials :warning:

:exclamation: There is a different number of reports uploaded between BASE (83b90fa) and HEAD (c3af69c). Click for more details.

HEAD has 4 uploads less than BASE

Flag	BASE (83b90fa)	HEAD (c3af69c)
	5	1

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2323      +/-   ##
==========================================
- Coverage   78.69%   71.62%   -7.08%     
==========================================
  Files         187      187              
  Lines       19257    19211      -46     
  Branches     4029     2784    -1245     
==========================================
- Hits        15155    13759    -1396     
- Misses       3404     4774    +1370     
+ Partials      698      678      -20     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[fellhorn]

@fellhorn fyi

Thanks, we need to ignore the Promise types though and thus anyway can't have type hints for the with_overrides :/ See https://github.com/flyteorg/flyte/issues/3682

It's either having type hints for the Promise types for us or having type checks for task & workflow inputs & outputs.

[pingsutw]

Thanks, we need to ignore the Promise types though and thus anyway can't have type hints for the with_overrides :/

@fellhorn could we merge this first and then figure out the type issue later? this will allow us to see the type hint in the IDE for with_override. Also, it cleans up the code. (remove if "container_image" in kwargs ...)

[fellhorn]

Thanks, we need to ignore the Promise types though and thus anyway can't have type hints for the with_overrides :/

@fellhorn could we merge this first and then figure out the type issue later? this will allow us to see the type hint in the IDE for with_override. Also, it cleans up the code. (remove if "container_image" in kwargs ...)

Sorry, I overlooked your message. Yes, please merge this PR. My answer to fg91 was rather about our internal flytekit wrapper, that helps us to check the runtime types instead of the graph types. I imagine my comment is not relevant for other flytekit users.