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

fix: remove unused setting in deployment charts #4252

Merged
merged 2 commits into from
Oct 30, 2023

Conversation

HeetVekariya
Copy link
Contributor

Tracking issue

Closes #4251


Describe your changes

  • Issue asks to remove the following code:
  name: grpc-2
  port: 8089
  protocol: TCP
  targetPort: 8089

Changes made: Removed above code


Files changed:

  • service.yaml (charts/flyte-core/templates/datacatalog/service.yaml)

Total changes: 4


Check all the applicable boxes

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

Screenshots

Note to reviewers

@welcome
Copy link

welcome bot commented Oct 17, 2023

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

  • Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
  • Sign off your commits (Reference: DCO Guide).

Copy link
Member

@pingsutw pingsutw left a comment

Choose a reason for hiding this comment

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

Thanks. could you run make helm and push it again

@HeetVekariya
Copy link
Contributor Author

HeetVekariya commented Oct 18, 2023

  • @pingsutw I have run the command make helm, and pushed the changes.

@Future-Outlier
Copy link
Member

@pingsutw, @HeetVekariya
You should run

make helm;make helm_update;

Not only just make helm.

@Future-Outlier
Copy link
Member

@HeetVekariya You should remember to sign off.

@HeetVekariya HeetVekariya force-pushed the fix/remove-unused-setting branch from 37311f9 to be4217a Compare October 18, 2023 09:45
@HeetVekariya
Copy link
Contributor Author

  • @Future-Outlier I am impressed, you have 100+ contribution in the flyte. 👍
  • Sorry for the inconvenience, when i visited this :https://github.com/flyteorg/flyte/blob/master/CONTRIBUTING.md there is only line, i think there is nothing in it (Not expacted a link for contribution.md) so just ignored it.
  • But now i realise that i have missed guild line.
  • All things corrected, ping me if any changes needed

@HeetVekariya
Copy link
Contributor Author

  • @Future-Outlier Can we connect on discord ?
  • If there is any discord server of flyte or like that.

@Future-Outlier
Copy link
Member

Future-Outlier commented Oct 18, 2023

@HeetVekariya Yes, you can join the Slack Channel, and feel free to direct message me!
My name is Han-Ru.
https://flyte-org.slack.com/ssb/redirect

Copy link
Member

@Future-Outlier Future-Outlier left a comment

Choose a reason for hiding this comment

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

LGTM!

@samhita-alla
Copy link
Contributor

hey @pingsutw, can we merge this PR? Does this look good to you?

@pingsutw pingsutw merged commit d4981f1 into flyteorg:master Oct 30, 2023
12 checks passed
@welcome
Copy link

welcome bot commented Oct 30, 2023

Congrats on merging your first pull request! 🎉

@HeetVekariya
Copy link
Contributor Author

Thank you for merging 👍

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

Successfully merging this pull request may close these issues.

[Housekeeping] Remove unused setting in deployment charts
5 participants