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

Docs: Add a section in Servers for setting extra capabilities #4718

Merged
merged 7 commits into from
Mar 10, 2023

Conversation

KengoA
Copy link
Contributor

@KengoA KengoA commented Mar 9, 2023

What this PR does / why we need it:

Add a section in Servers page explaining how to set extraCapabilities for servers and requirements for models.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

@KengoA KengoA requested a review from ukclivecox March 9, 2023 14:23
@agrski agrski self-requested a review March 9, 2023 14:59
Copy link
Contributor

@agrski agrski left a comment

Choose a reason for hiding this comment

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

We should probably revisit this page and add more detail here, as it's a bit brief overall.

This PR is a good step towards that 🚀

samples/models/extra-capabilities.yaml Outdated Show resolved Hide resolved
```{literalinclude} ../../../../samples/servers/mlserver-extra-capabilities.yaml
:language: yaml
```
Note that `serverConfig: mlserver` will provide default capabilities for MLServer as shown above, and the values specified in `extraCapabilities` are appended to them to create a single list of capabilities.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Note that `serverConfig: mlserver` will provide default capabilities for MLServer as shown above, and the values specified in `extraCapabilities` are appended to them to create a single list of capabilities.
Note that `serverConfig: mlserver` will provide default capabilities for MLServer as shown above, and the values specified in `extraCapabilities` are appended to them to create a single list of capabilities for this `Server`.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Note that `serverConfig: mlserver` will provide default capabilities for MLServer as shown above, and the values specified in `extraCapabilities` are appended to them to create a single list of capabilities.
This server, `mlserver-extra`, inherits a default set of capabilities via `serverConfig: mlserver`.
These defaults are discussed above.
The `extraCapabilities` are appended to these to create a single list of capabilities for this server.

Alternative suggestion that perhaps read better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes the second suggestion reads better. I added a new commit suggestion below myself as I couldn't apply it to the outdated file.

Co-authored-by: Alex Rakowski <20504869+agrski@users.noreply.github.com>
@KengoA KengoA merged commit f5a7b79 into SeldonIO:v2 Mar 10, 2023
@KengoA KengoA deleted the docs/server-with-extra-capabilities branch March 10, 2023 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants