Skip to content

Conversation

Sovietaced
Copy link
Member

@Sovietaced Sovietaced commented May 12, 2025

Why are the changes needed?

The metrics path is not configurable on the cluster resource manager so this simplifies the helm chart.

What changes were proposed in this pull request?

Remove the ability to configure the cluster resource manager metrics path since it is not effective.

Setup process

Screenshots

Check all the applicable boxes

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

Related PRs

Summary by Bito

This pull request simplifies the Helm chart for the cluster resource manager by removing ineffective metrics path configurations and replacing them with a static value. It also updates HA shared secret checksums across multiple manifest files for consistency, enhancing the clarity of the Helm templates.

@flyte-bot
Copy link
Collaborator

Code Review Agent Run Status

  • Limitations and other issues: ❌ Failure - Bito Automatic Review Skipped - Draft PR

    Bito didn't auto-review because this pull request is in draft status.
    To trigger review, mark the PR as ready or type /review in the comment and save.
    You can change draft PR review settings here, or contact the agent instance creator at [email protected].

@Sovietaced Sovietaced added the housekeeping Issues that help maintain flyte and keep it tech-debt free label May 12, 2025
@Sovietaced Sovietaced marked this pull request as ready for review May 12, 2025 11:54
Copy link

codecov bot commented May 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.48%. Comparing base (e54a0ac) to head (37ed0bb).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6444   +/-   ##
=======================================
  Coverage   58.48%   58.48%           
=======================================
  Files         940      940           
  Lines       71584    71584           
=======================================
+ Hits        41866    41867    +1     
+ Misses      26535    26534    -1     
  Partials     3183     3183           
Flag Coverage Δ
unittests-datacatalog 59.03% <ø> (ø)
unittests-flyteadmin 56.26% <ø> (ø)
unittests-flytecopilot 30.99% <ø> (ø)
unittests-flytectl 64.72% <ø> (ø)
unittests-flyteidl 76.12% <ø> (ø)
unittests-flyteplugins 60.95% <ø> (ø)
unittests-flytepropeller 54.78% <ø> (ø)
unittests-flytestdlib 64.04% <ø> (+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.

machichima
machichima previously approved these changes May 25, 2025
Copy link
Member

@machichima machichima left a comment

Choose a reason for hiding this comment

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

LGTM

@machichima
Copy link
Member

Hi @Sovietaced
Would you mind fixing the conflicts?

Signed-off-by: Jason Parraga <[email protected]>
@Sovietaced
Copy link
Member Author

Hi @Sovietaced
Would you mind fixing the conflicts?

Done

@Sovietaced Sovietaced enabled auto-merge (squash) May 25, 2025 17:50
@Sovietaced Sovietaced merged commit 8bf1de6 into flyteorg:master May 26, 2025
54 checks passed
@Sovietaced Sovietaced deleted the helm-cleanup branch May 26, 2025 01:51
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.

3 participants