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 issues with flink application list command #2909

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

Conversation

vsantwana
Copy link
Member

@vsantwana vsantwana commented Oct 17, 2024

Release Notes

Breaking Changes

  • PLACEHOLDER

New Features

  • Remove Environment from the flink application list command.

Bug Fixes

  • Fixes failures in the flink application list command if any of the attributes was null.

Checklist

  • Leave this box unchecked if features are not yet available in production

What

In the flink application list --output human command, we did not have null check on the properties we want to show in the application summary command. But this is a valid situation, when application is created, it can have jobName as null till it starts and same for status.

This PR adds check on those attributes.

Note: json and yaml output works fine

The other fix that we want to change in the same command is that we have removed spec.flinkEnvironment from flink application. The changes have to be made to the

  • Remove Environment from the confluent flink application list --environment env. The env is already there in the command.
  • Since flinkEnvironment will no longer be a part of the response of describe and list applications, it is removed from the test files.

References

Test & Review

@vsantwana vsantwana requested a review from a team as a code owner October 17, 2024 09:03
@confluent-cla-assistant
Copy link

🎉 All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

rmetzger
rmetzger previously approved these changes Oct 17, 2024
rmetzger
rmetzger previously approved these changes Oct 18, 2024
sgagniere
sgagniere previously approved these changes Oct 21, 2024
Copy link
Member

@sgagniere sgagniere left a comment

Choose a reason for hiding this comment

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

Looks good. Can you add a description under the bug fix section in the PR description?

Our release notes are auto generated from PR descriptions.

@airlock-confluentinc airlock-confluentinc bot dismissed stale reviews from sgagniere and rmetzger via f5ae20e October 22, 2024 05:56
@vsantwana vsantwana changed the title Fixes issue when application attributes are null Fix issues with flink application list command Oct 22, 2024
@@ -5,10 +5,9 @@ import (
)

type flinkApplicationSummaryOut struct {
Name string `human:"Name" serialized:"name"`
Environment string `human:"Environment" serialized:"environment"`
Copy link
Member

@sgagniere sgagniere Oct 24, 2024

Choose a reason for hiding this comment

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

I think this is a breaking change (environment).

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

Successfully merging this pull request may close these issues.

3 participants