Dtorresf/ch01s01

[diego-torres]

No description provided.

[github-actions]

PR Preview Action v1.4.4
Preview removed because the pull request was closed.
2023-11-21 01:00 UTC

[jramcast]

Good job Diego! I tested the exercises. They work fine. I left mostly format and style comments.

Is this section meant to be somewhere else? I am checking the design doc for course 4 but the contents of this PR do not completely align with the document?
I am assuming this section is unit 1 / section 1 as specified in the Design Doc

You might also make it clear that although we are actually serving a model with Flask in the exercise, that is not considered part of the Model Serving feature.

[rsriniva]

@diego-torres - please address review comments and push changes to same PR.

[rsriniva]

@mamurak - please review this first section in model deployment course

[jramcast]

I'd suggest you double check this with @rsriniva cause the main objective of this section is:

• Describe the concepts and terminology around model serving on RHODS

We can keep the "old-style" exercises to build that history line, that's a great idea actually.
However, we are still missing some of the terminology concepts in this section (e.g. what's a model server). Again, double check with @rsriniva .

[mamurak]

Hey @diego-torres, thanks for creating this section draft!
I added a couple of comments, mostly minor formal ones. The two main recommendations I have are:

• moving the code to the course git repository to be checked out by the student as they're doing in the other parts of the course,
• building the container image directly on OpenShift based on the git import strategy.

See review comments for details.

[diego-torres]

@jramcast , in commit 3052a17, I am describing more concepts from the design document.
A few topics I think will be better to explain in the next sections:

• Model Server Authentication
• GPU for model server

In the next section, while creating a model server with OpenVINO, will explain the options that are offered to configure service accounts that allow applications to authenticate to the model server, and how while creating the model server, there are options for the configured server sizes that can specify GPU.
But I consider that until the student is confronted with the options, the student will make better sense of what they mean.

Please review and let me know if additional modifications are required for this PR to be approved.

Thank you.

[rsriniva]

LGTM. Let's get this merged

[rsriniva]

@mamurak - I think this PR requires your approval. Please check Diego's update

[jramcast]

@jramcast , in commit 3052a17, I am describing more concepts from the design document. A few topics I think will be better to explain in the next sections:

• Model Server Authentication
• GPU for model server

In the next section, while creating a model server with OpenVINO, will explain the options that are offered to configure service accounts that allow applications to authenticate to the model server, and how while creating the model server, there are options for the configured server sizes that can specify GPU. But I consider that until the student is confronted with the options, the student will make better sense of what they mean.

Please review and let me know if additional modifications are required for this PR to be approved.

Thank you.

Good job @diego-torres . The changes look good to me. I assume the next section to explain OpenVINO will go in a separate PR, right?

[mamurak]

Looks good to me.

[diego-torres]

@jramcast , in commit 3052a17, I am describing more concepts from the design document. A few topics I think will be better to explain in the next sections:

• Model Server Authentication
• GPU for model server

In the next section, while creating a model server with OpenVINO, will explain the options that are offered to configure service accounts that allow applications to authenticate to the model server, and how while creating the model server, there are options for the configured server sizes that can specify GPU. But I consider that until the student is confronted with the options, the student will make better sense of what they mean.
Please review and let me know if additional modifications are required for this PR to be approved.
Thank you.

Good job @diego-torres . The changes look good to me. I assume the next section to explain OpenVINO will go in a separate PR, right?

OpenVino included here: #4