Skip to content

Conversation

Sovietaced
Copy link
Member

@Sovietaced Sovietaced commented Jun 29, 2025

What changes were proposed in this pull request?

Cleans up unused functions, variables, redundant conditions, unused variables, string format issues, redundant type declarations.

How was this patch tested?

Unit tests

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

Summary by Bito

This pull request enhances the Flyte propeller codebase by removing unused functions, variables, and redundant conditions, while addressing string format issues and redundant type declarations. These changes improve code readability and maintainability without altering existing functionality.

@flyte-bot
Copy link
Collaborator

Bito Automatic Review Skipped - Draft PR

Bito didn't auto-review because this pull request is in draft status.
No action is needed if you didn't intend for the agent to review it. Otherwise, to manually trigger a review, type /review in a comment and save.
You can change draft PR review settings here, or contact your Bito workspace admin at [email protected].

Copy link

codecov bot commented Jun 29, 2025

Codecov Report

❌ Patch coverage is 58.57143% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.60%. Comparing base (68fc4e6) to head (300a385).

Files with missing lines Patch % Lines
...eller/pkg/controller/nodes/array/event_recorder.go 42.85% 4 Missing ⚠️
...kg/controller/nodes/subworkflow/launchplan/noop.go 50.00% 2 Missing ⚠️
...er/pkg/controller/nodes/subworkflow/subworkflow.go 0.00% 2 Missing ⚠️
.../nodes/task/fakeplugins/next_phase_state_plugin.go 0.00% 2 Missing ⚠️
...lytepropeller/pkg/controller/nodes/task/handler.go 33.33% 2 Missing ⚠️
...nodes/task/resourcemanager/noop_resourcemanager.go 50.00% 2 Missing ⚠️
...propeller/pkg/controller/workflowstore/inmemory.go 0.00% 2 Missing ⚠️
flytepropeller/pkg/utils/failing_datastore.go 75.00% 2 Missing ⚠️
flytepropeller/events/local_writer.go 50.00% 1 Missing ⚠️
...pkg/apis/flyteworkflow/v1alpha1/workflow_status.go 0.00% 1 Missing ⚠️
... and 9 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6516      +/-   ##
==========================================
+ Coverage   58.58%   58.60%   +0.01%     
==========================================
  Files         929      929              
  Lines       70851    70832      -19     
==========================================
  Hits        41510    41510              
+ Misses      26194    26176      -18     
+ Partials     3147     3146       -1     
Flag Coverage Δ
unittests-datacatalog 59.03% <ø> (ø)
unittests-flyteadmin 56.12% <ø> (ø)
unittests-flytecopilot 39.56% <ø> (ø)
unittests-flytectl 64.64% <ø> (ø)
unittests-flyteidl 76.12% <ø> (ø)
unittests-flyteplugins 61.10% <ø> (ø)
unittests-flytepropeller 55.11% <58.57%> (+0.05%) ⬆️
unittests-flytestdlib 63.11% <ø> (ø)

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.

@Sovietaced Sovietaced force-pushed the propeller-cleanup branch 3 times, most recently from 990ae01 to 9d6aed8 Compare June 29, 2025 01:11
@flyte-bot
Copy link
Collaborator

Bito Automatic Review Skipped - Draft PR

Bito didn't auto-review because this pull request is in draft status.
No action is needed if you didn't intend for the agent to review it. Otherwise, to manually trigger a review, type /review in a comment and save.
You can change draft PR review settings here, or contact your Bito workspace admin at [email protected].

Signed-off-by: Jason Parraga <[email protected]>
@Sovietaced Sovietaced added the housekeeping Issues that help maintain flyte and keep it tech-debt free label Aug 30, 2025
Signed-off-by: Jason Parraga <[email protected]>
Signed-off-by: Jason Parraga <[email protected]>
@Sovietaced Sovietaced marked this pull request as ready for review August 30, 2025 21:34
@Sovietaced Sovietaced requested a review from machichima August 30, 2025 21:35

w := bufio.NewWriter(f)

if err != nil {
Copy link
Member Author

Choose a reason for hiding this comment

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

duplicate check

if err != nil {
return nil, err
}
if err != nil {
Copy link
Member Author

Choose a reason for hiding this comment

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

duplicate check

return configSection.GetConfig().(*Config)
}

func SetConfig(cfg *Config) error {
Copy link
Member Author

Choose a reason for hiding this comment

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

unused

workflowLauncher launchplan.Executor, launchPlanReader launchplan.Reader, defaultRawOutputPrefix storage.DataReference, kubeClient executors.Client,
catalogClient catalog.Client, recoveryClient recovery.Client, literalOffloadingConfig config.LiteralOffloadingConfig, eventConfig *config.EventConfig, clusterID string, signalClient service.SignalServiceClient,
nodeHandlerFactory interfaces.HandlerFactory, scope promutils.Scope) (interfaces.Node, error) {
func NewExecutor(ctx context.Context, nodeConfig config.NodeConfig, store *storage.DataStore, enQWorkflow v1alpha1.EnqueueWorkflow, eventSink events.EventSink, defaultRawOutputPrefix storage.DataReference, catalogClient catalog.Client, recoveryClient recovery.Client, literalOffloadingConfig config.LiteralOffloadingConfig, eventConfig *config.EventConfig, clusterID string, nodeHandlerFactory interfaces.HandlerFactory, scope promutils.Scope) (interfaces.Node, error) {
Copy link
Member Author

Choose a reason for hiding this comment

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

bunch of unused params here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

housekeeping Issues that help maintain flyte and keep it tech-debt free

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants