flytekit icon indicating copy to clipboard operation
flytekit copied to clipboard

Propagate custom_info Dict through agent Resource

Open ddl-ebrown opened this issue 1 year ago • 1 comments

See also https://github.com/flyteorg/flyte/pull/5391

Tracking issue

Why are the changes needed?

  • The agent defines a Resource return type with values:

    • outputs
    • message
    • log_links
    • phase

    These are all a part of the underlying protobuf contract defined in flyteidl.

    However, the message field custom_info from the protobuf is not here

    google.protobuf.Struct custom_info

    https://github.com/flyteorg/flyte/blob/519080b6e4e53fc0e216b5715ad9b5b5270f35c0/flyteidl/protos/flyteidl/admin/agent.proto#L140

    This field was added in https://github.com/flyteorg/flyte/pull/4874 but never made it into the corresponding flytekit PR https://github.com/flyteorg/flytekit/pull/2146

  • It's useful for agents to return additional metadata about the job, and it looks like custom_info is the intended location

  • Make a minor refactor to how the agent responds to requests that return Resource by implementing to_flyte_idl / from_flyte_idl directly

What changes were proposed in this pull request?

How was this patch tested?

Setup process

Screenshots

Check all the applicable boxes

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

Related PRs

Docs link

ddl-ebrown avatar May 16 '24 20:05 ddl-ebrown

Codecov Report

Attention: Patch coverage is 50.00000% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 77.62%. Comparing base (70332db) to head (2fd654a). Report is 1 commits behind head on master.

Files Patch % Lines
flytekit/extend/backend/base_agent.py 52.94% 8 Missing :warning:
flytekit/extend/backend/agent_service.py 33.33% 2 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2426      +/-   ##
==========================================
+ Coverage   72.10%   77.62%   +5.51%     
==========================================
  Files         181      238      +57     
  Lines       18395    20841    +2446     
  Branches     3601     3600       -1     
==========================================
+ Hits        13264    16178    +2914     
+ Misses       4506     4062     -444     
+ Partials      625      601      -24     

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

codecov[bot] avatar May 18 '24 22:05 codecov[bot]

Tracking issue: https://github.com/flyteorg/flyte/issues/5603

ddl-rliu avatar Jul 29 '24 18:07 ddl-rliu

@ddl-rliu Could you rebase the PR to fix the CI error?

pingsutw avatar Sep 05 '24 18:09 pingsutw

something wrong in the github Screenshot 2024-09-05 at 11 14 06 AM

pingsutw avatar Sep 05 '24 18:09 pingsutw

Fixed!

ddl-rliu avatar Sep 05 '24 18:09 ddl-rliu

@ddl-rliu overall lgtm, some tests are failing though.

pingsutw avatar Sep 09 '24 22:09 pingsutw

test errors should be fixed now 👍

ddl-rliu avatar Sep 17 '24 19:09 ddl-rliu