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

Remove lyft api dependency and migrate to lyft->flyteorg #157

Merged
merged 21 commits into from
Mar 11, 2021

Conversation

katrogan
Copy link
Contributor

@katrogan katrogan commented Mar 9, 2021

TL;DR

Remove lyft api dependency and migrate to lyft->flyteorg.

With upgrading the k8s libraries, cluster resource controller had to change significantly. The client-go interface only accepts client Objects now when using server side apply. This PR borrows heavily from the example here to implement a dynamic server side apply that doesn't require us to unmarshal the templatized yaml files to a known type to comply with the interface changes.

Tested locally on flyte sandbox

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

Tracking Issue

flyteorg/flyte#377

Follow-up issue

NA

EngHabu
EngHabu previously approved these changes Mar 9, 2021
Copy link
Contributor

@EngHabu EngHabu left a comment

Choose a reason for hiding this comment

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

🚢

@codecov
Copy link

codecov bot commented Mar 9, 2021

Codecov Report

Merging #157 (f664ec8) into master (0b778a9) will increase coverage by 1.36%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #157      +/-   ##
==========================================
+ Coverage   61.48%   62.84%   +1.36%     
==========================================
  Files         106      100       -6     
  Lines        7651     7261     -390     
==========================================
- Hits         4704     4563     -141     
+ Misses       2350     2118     -232     
+ Partials      597      580      -17     
Flag Coverage Δ
unittests 62.84% <75.00%> (+1.36%) ⬆️

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

Impacted Files Coverage Δ
pkg/async/notifications/email.go 100.00% <ø> (ø)
pkg/async/notifications/factory.go 0.00% <ø> (ø)
...async/notifications/implementations/aws_emailer.go 100.00% <ø> (ø)
...c/notifications/implementations/event_publisher.go 93.10% <ø> (ø)
...otifications/implementations/noop_notifications.go 0.00% <ø> (ø)
...g/async/notifications/implementations/processor.go 71.23% <ø> (ø)
...g/async/notifications/implementations/publisher.go 100.00% <ø> (ø)
pkg/async/schedule/aws/cloud_watch_scheduler.go 82.14% <ø> (ø)
pkg/async/schedule/aws/serialization.go 67.74% <ø> (ø)
pkg/async/schedule/aws/shared.go 55.55% <ø> (ø)
... and 89 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b778a9...f664ec8. Read the comment docs.

katrogan added 13 commits March 10, 2021 10:59
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
@katrogan katrogan force-pushed the flyteorg-bye-lyft branch from 7c5460a to a5c23c4 Compare March 10, 2021 18:59
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
@katrogan katrogan changed the title [wip] Remove lyft api dependency and migrate to lyft->flyteorg Remove lyft api dependency and migrate to lyft->flyteorg Mar 10, 2021
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
@katrogan katrogan merged commit c80c8d8 into master Mar 11, 2021
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