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

Refactor to ensure branch node rename is consistent in all nodes and connections #268

Merged
merged 14 commits into from
Jun 2, 2021

Conversation

EngHabu
Copy link
Contributor

@EngHabu EngHabu commented May 29, 2021

Signed-off-by: Haytham Abuelfutuh [email protected]

TL;DR

Refactor the part in the compiler that takes care of renaming node ids for sub-nodes.

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

Tracking Issue

flyteorg/flyte#1052

@codecov
Copy link

codecov bot commented May 29, 2021

Codecov Report

Merging #268 (03ba50b) into master (f06e98d) will increase coverage by 0.05%.
The diff coverage is 57.14%.

Katrina Rogan and others added 11 commits June 1, 2021 21:06
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Haytham Abuelfutuh <[email protected]>
Signed-off-by: Haytham Abuelfutuh <[email protected]>
Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Haytham Abuelfutuh <[email protected]>
Signed-off-by: Haytham Abuelfutuh <[email protected]>
Signed-off-by: Haytham Abuelfutuh <[email protected]>
Signed-off-by: Haytham Abuelfutuh <[email protected]>
Signed-off-by: Haytham Abuelfutuh <[email protected]>
Signed-off-by: Haytham Abuelfutuh <[email protected]>
@EngHabu EngHabu force-pushed the rename-upstream branch from 4bb40a4 to 0b6a55f Compare June 2, 2021 04:07
Signed-off-by: Haytham Abuelfutuh <[email protected]>
@EngHabu EngHabu changed the title Reflect node id change in upstream and downstream connections Refactor to ensure branch node rename is consistent in all nodes and connections Jun 2, 2021
@EngHabu EngHabu requested a review from kumare3 June 2, 2021 15:42
@EngHabu EngHabu marked this pull request as ready for review June 2, 2021 15:42
Signed-off-by: Haytham Abuelfutuh <[email protected]>
kumare3
kumare3 previously approved these changes Jun 2, 2021
Copy link
Contributor

@kumare3 kumare3 left a comment

Choose a reason for hiding this comment

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

lgtm

Signed-off-by: Haytham Abuelfutuh <[email protected]>
@EngHabu EngHabu merged commit 0437b71 into master Jun 2, 2021
eapolinario pushed a commit to eapolinario/flytepropeller that referenced this pull request Aug 9, 2023
…connections (flyteorg#268)

* Bump to pick up latest plugins (flyteorg#267)

Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Reflect node id change in upstream and downstream connections

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* wip

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* fix lint issue

Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Fix connections for branch nodes

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Support mismatching interfaces for branches

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* only fail adding nodes in compiler transformer when nodes are different

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* remove replaceNodeID

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Fix unit tests

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Clean up commented code

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* bump DCO

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Cleanup

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Revert config.yaml

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* PR Comments

Signed-off-by: Haytham Abuelfutuh <[email protected]>

Co-authored-by: Katrina Rogan <[email protected]>
Co-authored-by: Ketan Umare <[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.

2 participants