Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Address resolution #546

Merged
merged 16 commits into from
May 5, 2023
Merged

Address resolution #546

merged 16 commits into from
May 5, 2023

Conversation

wild-endeavor
Copy link
Contributor

@wild-endeavor wild-endeavor commented Mar 29, 2023

TL;DR

Add logic to return and accept flyte:// when getting data

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

How did you fix the bug, make the feature etc. Link to any design docs etc

Tracking Issue

Related to: flyteorg/flyteidl#391

Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
pkg/common/flyte_url.go Outdated Show resolved Hide resolved
pkg/common/flyte_url.go Outdated Show resolved Hide resolved
pkg/common/flyte_url.go Outdated Show resolved Hide resolved
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
@wild-endeavor wild-endeavor changed the title [wip] address resolution Address resolution May 1, 2023
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
@wild-endeavor wild-endeavor marked this pull request as ready for review May 2, 2023 02:09
@codecov
Copy link

codecov bot commented May 2, 2023

Codecov Report

Merging #546 (2a3054f) into master (1026231) will increase coverage by 1.64%.
The diff coverage is 72.18%.

❗ Current head 2a3054f differs from pull request most recent head 9f919a2. Consider uploading reports for the commit 9f919a2 to get more accurate results

@@            Coverage Diff             @@
##           master     #546      +/-   ##
==========================================
+ Coverage   58.85%   60.49%   +1.64%     
==========================================
  Files         170      172       +2     
  Lines       16068    13303    -2765     
==========================================
- Hits         9457     8048    -1409     
+ Misses       5777     4409    -1368     
- Partials      834      846      +12     
Flag Coverage Δ
unittests ?

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

Impacted Files Coverage Δ
pkg/common/artifacttype_enumer.go 40.00% <40.00%> (ø)
dataproxy/service.go 57.71% <63.09%> (+4.84%) ⬆️
pkg/common/flyte_url.go 96.00% <96.00%> (ø)
pkg/manager/impl/node_execution_manager.go 72.70% <100.00%> (+2.70%) ⬆️
pkg/manager/impl/task_execution_manager.go 72.39% <100.00%> (+3.10%) ⬆️

... and 153 files with indirect coverage changes

dataproxy/service.go Outdated Show resolved Hide resolved
taskExecs, err := s.taskExecutionManager.ListTaskExecutions(ctx, admin.TaskExecutionListRequest{
NodeExecutionId: &nodeExecID,
Limit: 1,
Filters: fmt.Sprintf("eq(retry_attempt,%s)", strconv.Itoa(attempt)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a strongly typed way of doing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really... i think we should punt on this.

dataproxy/service.go Show resolved Hide resolved
pkg/common/flyte_url.go Show resolved Hide resolved
wild-endeavor and others added 2 commits May 3, 2023 17:40
Co-authored-by: Haytham Abuelfutuh <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
EngHabu
EngHabu previously approved these changes May 5, 2023
pkg/common/flyte_url.go Outdated Show resolved Hide resolved
Co-authored-by: Haytham Abuelfutuh <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
@wild-endeavor wild-endeavor merged commit 950ad15 into master May 5, 2023
@wild-endeavor wild-endeavor deleted the resolve branch May 5, 2023 21:58
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
Signed-off-by: Yee Hing Tong <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants