Skip to content

Commit

Permalink
Merge branch 'main' into f/mserving-metrics
Browse files Browse the repository at this point in the history
  • Loading branch information
alexcreasy committed Sep 21, 2023
2 parents 4656676 + 31b70b7 commit 3ee7a87
Show file tree
Hide file tree
Showing 104 changed files with 2,712 additions and 6,603 deletions.
64 changes: 64 additions & 0 deletions .github/ISSUE_TEMPLATE/internal_tracker.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
name: (Internal) Tracker Template
description: Intended to help with a template for tracking larger grouped items.
title: "[Tracker]: "
labels: ["tracker"]
body:
- type: textarea
id: description
attributes:
label: Description
description: A introductory description of the larger task
validations:
required:
true
- type: input
id: branch
attributes:
label: Target Branch
description: What is the feature branch to contain this effort? If not known at this time, replace with `TBD`
placeholder: f/
validations:
required: true
- type: textarea
id: requirements
attributes:
label: Requirements
description: A series of requirements to consider this tracker complete.
placeholder: |
* P0: Show something
* P2: Allow users to change permissions
validations:
required: true
- type: textarea
id: ux-issues
attributes:
label: Itemized UX Issues
description: |
List the tickets that UX will work on.
Tip: Using a bullet list will help display links to other tickets by unraveling the name and status of that ticket.
placeholder: |
* #1234
* Design mocks - Ticket TBD
validations:
required: true
- type: textarea
id: dev-issues
attributes:
label: Itemized Dev Issues
description: |
List the tickets that Development will work on. If unknown at this time, add `TBD`
Tip: Using a bullet list will help display links to other tickets by unraveling the name and status of that ticket.
placeholder: |
* #1234
* Implement Table Page - Ticket TBD
validations:
required: true
- type: textarea
id: artifacts
attributes:
label: Related artifacts
description: Any additional artifacts that will help with the tracker goals
validations:
required: false
53 changes: 53 additions & 0 deletions .github/ISSUE_TEMPLATE/internal_ux.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
name: (Internal) UX Template
description: Intended to help ux create internal flows.
title: "[UX]: "
labels: ["kind/ux"]
body:
- type: textarea
id: description
attributes:
label: Description
description: A introductory description of the task
validations:
required:
true
- type: textarea
id: goals
attributes:
label: Goals
description: An itemized list of goals to complete for this ticket
placeholder: |
* Research...
* Design...
validations:
required: false
- type: textarea
id: output
attributes:
label: Expected Output
description: What would be considered the end result?
validations:
required: false
- type: textarea
id: related-issues
attributes:
label: Related Issues
description: |
Any related issues that might be useful to mention as it relates to this ticket's goals, expectations, or follow ups.
Tip: Using a bullet list will help display links to other tickets by unraveling the name and status of that ticket.
placeholder: |
* #1234
* Create figma designs - Ticket TBD
validations:
required: false
- type: textarea
id: artifacts
attributes:
label: Completed artifacts
description: |
Any artifacts you want to easily note as results of the effort.
Typically this is left empty at the start. Also useful to include useful links or information you would like to share for additional context.
validations:
required: false
2 changes: 1 addition & 1 deletion .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Self checklist (all need to be checked):
If you have UI changes:
<!--- You can ignore these if you are doing manifest, backend, internal logic, etc changes; aka non-UI / visual changes -->
- [ ] Included any necessary screenshots or gifs if it was a UI change.
- [ ] Included tags to the UX team if it was a UI/UX change.
- [ ] Included tags to the UX team if it was a UI/UX change (find relevant UX in the [SMEs](https://github.com/opendatahub-io/odh-dashboard/tree/main/docs/smes.md) section).

After the PR is posted & before it merges:
- [ ] The developer has tested their solution on a cluster by using the image produced by the PR to `main`
2 changes: 1 addition & 1 deletion .github/workflows/pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
steps:
- uses: actions/checkout@v3
- name: Setup Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v3.7.0
uses: actions/setup-node@v3.8.1
with:
node-version: ${{ matrix.node-version }}
- name: Node.js modules cache, repository
Expand Down
15 changes: 12 additions & 3 deletions OWNERS
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
approvers:
- andrewballantyne
- lucferbux
- alexcreasy
- christianvogt

reviewers:
- DaoDaoNoCode
- lucferbux
- Gkrumbach07
- alexcreasy
- christianvogt
- uidoyen
- Gkrumbach07
- lucferbux
- DaoDaoNoCode
- manaswinidas
- pnaik1
- ppadti
- dpanshug
4 changes: 4 additions & 0 deletions backend/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@
"settings": {
},
"rules": {
"no-restricted-properties": [ "error", {
"property": "toString",
"message": "e.toString() should be fastify.log.error(e, 'your string'). Other use-cases should avoid obj.toString() on principle. Craft the string you want instead."
}],
"@typescript-eslint/explicit-function-return-type": "off",
"@typescript-eslint/interface-name-prefix": "off",
"@typescript-eslint/no-var-requires": "off",
Expand Down
5 changes: 4 additions & 1 deletion backend/src/plugins/kube.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ export default fp(async (fastify: FastifyInstance) => {
);
clusterID = (clusterVersion.body as { spec: { clusterID: string } }).spec.clusterID;
} catch (e) {
fastify.log.error(`Failed to retrieve cluster id: ${e.response?.body?.message || e.message}.`);
fastify.log.error(
e,
`Failed to retrieve cluster id: ${e.response?.body?.message || e.message}.`,
);
}
let clusterBranding = 'okd';
try {
Expand Down
10 changes: 4 additions & 6 deletions backend/src/routes/api/cluster-settings/clusterSettingsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,7 @@ export const updateClusterSettings = async (
}
return { success: true, error: null };
} catch (e) {
fastify.log.error(
'Setting cluster settings error: ' + e.toString() + e.response?.body?.message,
);
fastify.log.error(e, 'Setting cluster settings error: ' + e.response?.body?.message);
if (e.response?.statusCode !== 404) {
return { success: false, error: 'Unable to update cluster settings. ' + e.message };
}
Expand All @@ -137,7 +135,7 @@ export const getClusterSettings = async (
clusterSettings.userTrackingEnabled =
segmentEnabledRes.body.data.segmentKeyEnabled === 'true';
} catch (e) {
fastify.log.error('Error retrieving segment key enabled: ' + e.toString());
fastify.log.error(e, 'Error retrieving segment key enabled.');
}
}
if (isJupyterEnabled) {
Expand Down Expand Up @@ -165,7 +163,7 @@ export const getClusterSettings = async (
if (e.statusCode === 404) {
fastify.log.warn('Notebook controller culling config not found, culling disabled...');
} else {
fastify.log.error('Error getting notebook controller culling settings: ' + e.toString());
fastify.log.error(e, 'Error getting notebook controller culling settings.');
throw e;
}
});
Expand All @@ -175,7 +173,7 @@ export const getClusterSettings = async (
clusterSettings.pvcSize = pvcSize;
clusterSettings.cullerTimeout = cullerTimeout;
} catch (e) {
fastify.log.error('Error retrieving cluster settings: ' + e.toString());
fastify.log.error(e, 'Error retrieving cluster settings.');
}
}

Expand Down
2 changes: 2 additions & 0 deletions backend/src/routes/api/dev-impersonate/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export default async (fastify: KubeFastifyInstance): Promise<void> => {
url,
{
headers: {
// This usage of toString is fine for internal dev flows
// eslint-disable-next-line no-restricted-properties
Authorization: `Basic ${Buffer.from(
`${DEV_IMPERSONATE_USER}:${DEV_IMPERSONATE_PASSWORD}`,
).toString('base64')}`,
Expand Down
4 changes: 2 additions & 2 deletions backend/src/routes/api/groups-config/groupsConfigUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const getGroupsConfig = async (fastify: KubeFastifyInstance): Promise<Gro

return groupsConfigProcessed.groupsConfig;
} catch (e) {
fastify.log.error('Error retrieving group configuration: ' + e.toString());
fastify.log.error(e, 'Error retrieving group configuration.');
const error = createError(500, 'Error retrieving group configuration');
throw error;
}
Expand Down Expand Up @@ -74,7 +74,7 @@ export const updateGroupsConfig = async (
error: null,
};
} catch (e) {
fastify.log.error('Error updating group configuration' + e.toString());
fastify.log.error(e, 'Error updating group configuration.');
const error = createError(500, 'Error updating group configuration');
throw error;
}
Expand Down
8 changes: 4 additions & 4 deletions backend/src/routes/api/images/imageUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ export const postImage = async (
return { success: true, error: null };
} catch (e) {
if (e.response?.statusCode !== 404) {
fastify.log.error('Unable to add notebook image: ' + e.toString());
fastify.log.error(e, 'Unable to add notebook image');
return { success: false, error: 'Unable to add notebook image: ' + e.message };
}
}
Expand Down Expand Up @@ -306,7 +306,7 @@ export const deleteImage = async (
return { success: true, error: null };
} catch (e) {
if (e.response?.statusCode !== 404) {
fastify.log.error('Unable to delete notebook image: ' + e.toString());
fastify.log.error(e, 'Unable to delete notebook image.');
return { success: false, error: 'Unable to delete notebook image: ' + e.message };
}
}
Expand All @@ -333,7 +333,7 @@ export const updateImage = async (
);

if (validName.length > 0) {
fastify.log.error('Duplicate name unable to add notebook image');
fastify.log.error('Duplicate name unable to add notebook image.');
return { success: false, error: 'Unable to add notebook image: ' + body.name };
}

Expand Down Expand Up @@ -397,7 +397,7 @@ export const updateImage = async (
return { success: true, error: null };
} catch (e) {
if (e.response?.statusCode !== 404) {
fastify.log.error('Unable to update notebook image: ' + e.toString());
fastify.log.error(e, 'Unable to update notebook image.');
return { success: false, error: 'Unable to update notebook image: ' + e.message };
}
}
Expand Down
7 changes: 5 additions & 2 deletions backend/src/routes/api/nb-events/eventUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,18 @@ import { KubeFastifyInstance } from '../../../types';
export const getNotebookEvents = async (
fastify: KubeFastifyInstance,
namespace: string,
podUID: string,
notebookName: string,
podUID: string | undefined,
): Promise<V1Event[]> => {
return fastify.kube.coreV1Api
.listNamespacedEvent(
namespace,
undefined,
undefined,
undefined,
`involvedObject.kind=Pod,involvedObject.uid=${podUID}`,
podUID
? `involvedObject.kind=Pod,involvedObject.uid=${podUID}`
: `involvedObject.kind=StatefulSet,involvedObject.name=${notebookName}`,
)
.then((res) => {
const body = res.body as V1EventList;
Expand Down
35 changes: 16 additions & 19 deletions backend/src/routes/api/nb-events/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,21 @@ import { getNotebookEvents } from './eventUtils';
import { secureRoute } from '../../../utils/route-security';

export default async (fastify: FastifyInstance): Promise<void> => {
fastify.get(
'/:namespace/:podUID',
secureRoute(fastify)(
async (
request: FastifyRequest<{
Params: {
namespace: string;
podUID: string;
};
Querystring: {
// TODO: Support server side filtering
from?: string;
};
}>,
) => {
const { namespace, podUID } = request.params;
return getNotebookEvents(fastify, namespace, podUID);
},
),
const routeHandler = secureRoute(fastify)(
async (
request: FastifyRequest<{
Params: {
namespace: string;
notebookName: string;
podUID: string | undefined;
};
}>,
) => {
const { namespace, notebookName, podUID } = request.params;
return getNotebookEvents(fastify, namespace, notebookName, podUID);
},
);

fastify.get('/:namespace/:notebookName', routeHandler);
fastify.get('/:namespace/:notebookName/:podUID', routeHandler);
};
2 changes: 1 addition & 1 deletion backend/src/routes/api/segment-key/segmentKeyUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export const getSegmentKey = async (fastify: KubeFastifyInstance): Promise<ODHSe
segmentKeyEnabled = resEnabled?.body.data?.segmentKeyEnabled === 'true';
if (segmentKeyEnabled) {
const res = await coreV1Api.readNamespacedSecret('odh-segment-key', namespace);
decodedSegmentKey = Buffer.from(res.body.data.segmentKey, 'base64').toString();
decodedSegmentKey = String(Buffer.from(res.body.data.segmentKey, 'base64'));
} else {
decodedSegmentKey = '';
}
Expand Down
16 changes: 14 additions & 2 deletions backend/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,25 @@ const transport =
: undefined;

const app = fastify({
logger: pino({ level: LOG_LEVEL }, transport),
logger: pino(
{
level: LOG_LEVEL,
redact: [
'err.response.request.headers.Authorization',
'response.request.headers.Authorization',
'request.headers.Authorization',
'headers.Authorization',
'Authorization',
],
},
transport,
),
pluginTimeout: 10000,
});

app.register(initializeApp);

app.listen({ port: PORT, host: IP}, (err) => {
app.listen({ port: PORT, host: IP }, (err) => {
if (err) {
app.log.error(err);
process.exit(1); // eslint-disable-line
Expand Down
2 changes: 2 additions & 0 deletions backend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -903,8 +903,10 @@ export type ServingRuntime = K8sResourceCommon & {
image: string;
name: string;
resources: ContainerResources;
volumeMounts?: VolumeMount[];
}[];
supportedModelFormats: SupportedModelFormats[];
replicas: number;
volumes?: Volume[];
};
};
Loading

0 comments on commit 3ee7a87

Please sign in to comment.