Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve readability for ingress.yaml under charts/flyte-core/templates/common #4945

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

lowc1012
Copy link
Contributor

Tracking issue

Closes #4944

Why are the changes needed?

Because charts/flyte-core/templates/common/ingress.yaml looks messy, it need to be improved for readability

What changes were proposed in this pull request?

  1. move grpcRoutes template to _helpers.tpl
  2. polish ingress.yaml and separate manifest into ingress.yaml & ingress-separateGrpc.yaml
  3. set default .Values.common.ingress.host and ingressClassName as empty string

How was this patch tested?

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

Docs link

@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. documentation Improvements or additions to documentation enhancement New feature or request labels Feb 24, 2024
Copy link

codecov bot commented Feb 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.00%. Comparing base (fd42f65) to head (27da5c2).
Report is 21 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4945   +/-   ##
=======================================
  Coverage   58.99%   59.00%           
=======================================
  Files         645      645           
  Lines       55542    55548    +6     
=======================================
+ Hits        32769    32775    +6     
  Misses      20182    20182           
  Partials     2591     2591           
Flag Coverage Δ
unittests 59.00% <ø> (+<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.

@lowc1012 lowc1012 force-pushed the flyte-issues-4944 branch from 97d62b8 to 97e6112 Compare March 4, 2024 11:19
lowc1012 added 4 commits March 7, 2024 18:50
Signed-off-by: Ryan Lo <[email protected]>
Signed-off-by: Ryan Lo <[email protected]>
Signed-off-by: Ryan Lo <[email protected]>
Signed-off-by: Ryan Lo <[email protected]>
@lowc1012 lowc1012 force-pushed the flyte-issues-4944 branch from 97e6112 to 20f59d0 Compare March 7, 2024 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Housekeeping] Improve readability for ingress.yaml under charts/flyte-core/templates/common
3 participants