Skip to content

[WIP][Flyteadmin] Add variablemap in dataproxy for dataclass/pydantic#6136

Closed
mao3267 wants to merge 4 commits intoflyteorg:masterfrom
mao3267:add-dataproxy-dataclass-variablemap
Closed

[WIP][Flyteadmin] Add variablemap in dataproxy for dataclass/pydantic#6136
mao3267 wants to merge 4 commits intoflyteorg:masterfrom
mao3267:add-dataproxy-dataclass-variablemap

Conversation

@mao3267
Copy link
Contributor

@mao3267 mao3267 commented Jan 4, 2025

Current Problem

There are two main components to manage when retrieving the variable_map in the Dataproxy's GetData function: GetDataFromNodeExecution and GetDataFromTaskExecution. Since the variable_map resides in the TaskClosure, it is logical to retrieve it from the Task Repository using the Task's identification details (ID, project, domain, and version).

However, I am unable to determine how to access the output from node execution. Is there a way to retrieve the TaskClosure (or Task) directly from the NodeExecution?

Tracking issue

#6081

Why are the changes needed?

While the task input/output is represented as a dataclass or Pydantic model, using the get function to fetch the FlyteRemote execution output will fail due to the absence of the variable_map information. To address this issue, we aim to provide the input/output variable_map through the Dataproxy as a solution.

What changes were proposed in this pull request?

  1. Provide variable_map information in GetData from dataproxy service.
  2. Unit tests will be added later.

How was this patch tested?

TODO

Setup process

git clone https://github.com/flyteorg/flyte.git
gh pr checkout 

Screenshots

TODO

Check all the applicable boxes

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

Related PRs

flyteorg/flytekit#3031

Docs link

TODO

Signed-off-by: mao3267 <chenvincent610@gmail.com>
Signed-off-by: mao3267 <chenvincent610@gmail.com>
@flyte-bot
Copy link
Collaborator

Code Review Agent Run Status

  • Limitations and other issues: ❌ Failure - The AI Code Review Agent skipped reviewing this change because it is configured to exclude certain pull requests based on the source/target branch or the pull request status. You can change the settings here, or contact the agent instance creator at eduardo@union.ai.

@codecov
Copy link

codecov bot commented Jan 4, 2025

Codecov Report

Attention: Patch coverage is 31.11111% with 93 lines in your changes missing coverage. Please review.

Project coverage is 37.05%. Comparing base (c20cd47) to head (76f7f28).
Report is 125 commits behind head on master.

Files with missing lines Patch % Lines
...teadmin/pkg/manager/impl/node_execution_manager.go 18.84% 56 Missing ⚠️
...eidl/gen/pb-go/flyteidl/admin/node_execution.pb.go 0.00% 10 Missing ⚠️
...eidl/gen/pb-go/flyteidl/admin/task_execution.pb.go 0.00% 10 Missing ⚠️
...teadmin/pkg/manager/impl/task_execution_manager.go 74.19% 6 Missing and 2 partials ⚠️
...lyteidl/gen/pb-go/flyteidl/service/dataproxy.pb.go 0.00% 5 Missing ⚠️
flyteadmin/dataproxy/service.go 75.00% 2 Missing ⚠️
...ts/go/admin/mocks/isGetDataResponse_VariableMap.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6136      +/-   ##
==========================================
- Coverage   37.06%   37.05%   -0.02%     
==========================================
  Files        1318     1319       +1     
  Lines      132638   132763     +125     
==========================================
+ Hits        49157    49190      +33     
- Misses      79231    79321      +90     
- Partials     4250     4252       +2     
Flag Coverage Δ
unittests-datacatalog 51.58% <ø> (ø)
unittests-flyteadmin 54.25% <38.88%> (-0.09%) ⬇️
unittests-flytecopilot 30.99% <ø> (ø)
unittests-flytectl 62.29% <ø> (ø)
unittests-flyteidl 7.23% <0.00%> (-0.01%) ⬇️
unittests-flyteplugins 53.85% <ø> (ø)
unittests-flytepropeller 42.69% <ø> (ø)
unittests-flytestdlib 55.29% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Future-Outlier Future-Outlier self-assigned this Jan 13, 2025
Signed-off-by: mao3267 <chenvincent610@gmail.com>
@flyte-bot
Copy link
Collaborator

Code Review Agent Run Status

  • Limitations and other issues: ❌ Failure - The AI Code Review Agent skipped reviewing this change because it is configured to exclude certain pull requests based on the source/target branch or the pull request status. You can change the settings here, or contact the agent instance creator at eduardo@union.ai.

@Future-Outlier
Copy link
Member

this is low priority and we should close it, sorry for telling you to do it.

@mao3267 mao3267 closed this Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants