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: adds an additional printer column annotation #6390

Merged
merged 3 commits into from
Oct 18, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
updating to jsonPath
Signed-off-by: Steve Hawkins <[email protected]>
shawkins committed Oct 16, 2024
commit 5b79029888bbab123d964d4798aa32b74feecc24
Original file line number Diff line number Diff line change
@@ -46,7 +46,7 @@ void addPrinterColumn(String path, String column, String format,

protected void handlePrinterColumns(AbstractJsonSchema<?, ?> resolver, PrinterColumnHandler handler) {
TreeMap<String, AnnotationMetadata> sortedCols = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
resolver.getAdditionalPrinterColumns().forEach(apc -> sortedCols.put(apc.path(), new AnnotationMetadata(apc, null)));
resolver.getAdditionalPrinterColumns().forEach(apc -> sortedCols.put(apc.jsonPath(), new AnnotationMetadata(apc, null)));
sortedCols.putAll(resolver.getAllPaths(PrinterColumn.class));
sortedCols.forEach((path, property) -> {
if (property.annotation instanceof AdditionalPrinterColumn) {
Copy link
Contributor

Choose a reason for hiding this comment

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

At the moment the @AdditionalPrinterColum is collected from everywhere in the spec or status type. Is this intended?

We should either clarify this in the description of the annotation or pick it only from the custom resource class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment the @AdditionalPrinterColum is collected from everywhere in the spec or status type.

It should not be: https://github.com/fabric8io/kubernetes-client/pull/6390/files#diff-e5491c474236ab12f8e6f5d9895464ecf77754b17ddb0e182682add6db8620ceR146

We should either clarify this in the description of the annotation or pick it only from the custom resource class.

It is documented as intended for the root: https://github.com/fabric8io/kubernetes-client/pull/6390/files#diff-669d381122146d717c360f17fcfe5eeaab88b5c84dd4bfcf81b6de71aa5a75c1R25

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, seems like it was a problem on my side. I have checked out your branch again and could not reproduce it. Looks fine now!

Original file line number Diff line number Diff line change
@@ -26,7 +26,7 @@
@Group("samples.javaoperatorsdk.io")
@Version("v1alpha1")
@ShortNames("jr")
@AdditionalPrinterColumn(name = "Age", path = ".metadata.creationTimestamp", type = Type.DATE)
@AdditionalPrinterColumn(name = "Age", jsonPath = ".metadata.creationTimestamp", type = Type.DATE)
public class JokeRequest extends CustomResource<JokeRequestSpec, JokeRequestStatus> implements Namespaced {

}
Original file line number Diff line number Diff line change
@@ -102,7 +102,7 @@ public String getValue() {
*
* @return
*/
String path();
String jsonPath();

/**
* The type of the printer column