chore: update examples node fetch #1074

clintonmedbery · 2023-05-04T16:52:33Z

This fixes a few of the examples in the FETCH_MIGRATION markdown file. The ones I have checked off I have tested. I have also updated a few more I will include in another PR that I have not been able to successfully test because of this issue:
#893

I also added the typescript examples to the document, as they will need to be upgraded as well.

examples/example.js

mstruebing · 2023-05-04T16:57:42Z

examples/in-cluster.js

    .then((res) => {
-	console.log(res.body);
+        console.log(res);


Is there a reason you've switched that to the complete res object?

I didn't. That is what this function is now returning. This example now reflects that.

examples/ingress.js

mstruebing · 2023-05-04T17:06:49Z

src/cache.ts

@@ -25,7 +25,7 @@ export class ListWatch<T extends KubernetesObject> implements ObjectCache<T>, In
    private objects: T[] = [];
    private resourceVersion: string;
    private readonly indexCache: { [key: string]: T[] } = {};
-    private readonly callbackCache: { [key: string]: (ObjectCallback<T> | ErrorCallback)[] } = {};
+    private readonly callbackCache: { [key: string]: Array<ObjectCallback<T> | ErrorCallback> } = {};


Could you explain why you need to make these changes in this file and the corresponding test?

Functions like k8sApi.listPodForAllNamespaces() no longer return the HTTP status and body. They return just the KubernetesObjects. I updated ListPromise to reflect this and updated the tests. Let me know if this is out of line, but the example and tests do not work otherwise.

clintonmedbery · 2023-05-04T17:17:31Z

FETCH_MIGRATION.md

    -   [ ] yaml-example

+-   [ ] Fix TypeScript examples and validate their param signatures (due to new api)


We didn't account for the TypeScript examples before.

clintonmedbery · 2023-05-04T17:17:54Z

examples/cache-example.js

@@ -8,7 +8,7 @@ const k8sApi = kc.makeApiClient(k8s.CoreV1Api);
 const path = '/api/v1/pods';
 const watch = new k8s.Watch(kc);

-const listFn = () => k8sApi.listPodForAllNamespaces()
+const listFn = () => k8sApi.listPodForAllNamespaces();


Some of these examples got formatted with prettier.

clintonmedbery · 2023-05-04T18:07:56Z

src/informer.ts

 export type ObjectCallback<T extends KubernetesObject> = (obj: T) => void;
 export type ErrorCallback = (err?: any) => void;
 export type ListCallback<T extends KubernetesObject> = (list: T[], ResourceVersion: string) => void;
-export type ListPromise<T extends KubernetesObject> = () => Promise<{


When running the example I noticed k8sApi.listPodForAllNamespaces() no longer returns a response and body, just the objects. I updated this accordingly.

mstruebing

👍 LGTM

brendandburns · 2023-05-05T18:37:22Z

examples/top_pods.js

@@ -1,39 +1,35 @@
-const k8s = require('../dist/index');
+const k8s = require('@kubernetes/client-node');


this needs to be removed b/c otherwise we are testing against the published package vs HEAD

Most of the examples use this. I will updated the others to use HEAD to match your comment.

Now I'm actually conflicted :) Perhaps it is better to have a working example? The trouble with not using HEAD is that if we change the API then we don't have continuous validation that the examples are working...

Maybe we can add a comment to each of these that says:
// in a real program use require('@kubernetes/client-node')

or something like that.

Ok, the rest of these seem to use require('@kubernetes/client-node');. I will make them use require('../dist/index'); but maybe it would be helpful to have a comment on how to import the package? Or maybe the readme does that already. Thinking out loud here. For now I will just import them.

Ok I just reloaded this page and saw your comment 😂. Sounds like we are on the same page. I will add a comment and make it clear.

brendandburns · 2023-05-05T18:38:00Z

One minor comment.

Also fixed a bug with ListPromise still returning body and response.

brendandburns · 2023-05-09T16:13:30Z

/lgtm
/approve

Thanks!

k8s-ci-robot · 2023-05-09T16:13:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brendandburns, clintonmedbery, mstruebing

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [brendandburns]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 4, 2023

k8s-ci-robot requested review from brendandburns and drubin May 4, 2023 16:52

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 4, 2023

mstruebing reviewed May 4, 2023

View reviewed changes

clintonmedbery commented May 4, 2023

View reviewed changes

clintonmedbery force-pushed the clintonmedbery/chore/update-examples-node-fetch branch 3 times, most recently from 86fd68c to 1fbfc18 Compare May 4, 2023 20:16

mstruebing approved these changes May 5, 2023

View reviewed changes

brendandburns reviewed May 5, 2023

View reviewed changes

clintonmedbery force-pushed the clintonmedbery/chore/update-examples-node-fetch branch from 1fbfc18 to 88f31f2 Compare May 8, 2023 00:43

chore: update examples for node fetch

34c6010

Also fixed a bug with ListPromise still returning body and response.

clintonmedbery force-pushed the clintonmedbery/chore/update-examples-node-fetch branch from 88f31f2 to 34c6010 Compare May 8, 2023 00:47

k8s-ci-robot assigned brendandburns May 9, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 9, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 9, 2023

k8s-ci-robot merged commit 15a3345 into kubernetes-client:release-1.x May 9, 2023
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: update examples node fetch #1074

chore: update examples node fetch #1074

clintonmedbery commented May 4, 2023

mstruebing May 4, 2023

clintonmedbery May 4, 2023

mstruebing May 4, 2023

clintonmedbery May 4, 2023

clintonmedbery May 4, 2023

clintonmedbery May 4, 2023

clintonmedbery May 4, 2023

mstruebing left a comment

brendandburns May 5, 2023

clintonmedbery May 5, 2023

brendandburns May 7, 2023

clintonmedbery May 8, 2023 •

edited

brendandburns commented May 5, 2023

brendandburns commented May 9, 2023

k8s-ci-robot commented May 9, 2023

		- [ ] yaml-example

		- [ ] Fix TypeScript examples and validate their param signatures (due to new api)

		@@ -1,39 +1,35 @@
		const k8s = require('../dist/index');
		const k8s = require('@kubernetes/client-node');

chore: update examples node fetch #1074

chore: update examples node fetch #1074

Conversation

clintonmedbery commented May 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mstruebing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clintonmedbery May 8, 2023 • edited

Choose a reason for hiding this comment

brendandburns commented May 5, 2023

brendandburns commented May 9, 2023

k8s-ci-robot commented May 9, 2023

clintonmedbery May 8, 2023 •

edited