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(api): Add additional docs to describe behaviour of model redeployment #478

Merged
merged 2 commits into from
Oct 25, 2023

Conversation

deadlycoconuts
Copy link
Contributor

What this PR does / why we need it:
This PR introduces some additional user documentation to describe the new behaviour when redeploying model versions in Merlin.

Which issue(s) this PR fixes:
None

Fixes #

Does this PR introduce a user-facing change?:

NONE

Checklist

  • Added unit test, integration, and/or e2e tests
  • Tested locally
  • Updated documentation
  • Update Swagger spec if the PR introduce API changes
  • Regenerated Golang and Python client if the PR introduce API changes

@deadlycoconuts deadlycoconuts added the documentation Improvements or additions to documentation label Oct 25, 2023
becomes the current *deployment* of the model version.

If the deployment of the new configuration fails, **the old deployment stays deployed** and remains as the current
*deployment* of the model version. The new configuration will then show a 'Failed' status.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm thinking of adding some screenshots of the UI here to show the new deployment history panel but I don't think it's absolutely necessary here because the rest of the docs don't really show screenshots of the UI. We can still do this for a start and add UI screenshots for the rest of the docs subsequently too. What do you all think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think one ss here here will do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll see what I can find and add one here.

@ghost
Copy link

ghost commented Oct 25, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@deadlycoconuts deadlycoconuts changed the title Add additional docs to describe behaviour of model redeployment docs(api): Add additional docs to describe behaviour of model redeployment Oct 25, 2023
@deadlycoconuts deadlycoconuts marked this pull request as ready for review October 25, 2023 10:02
@deadlycoconuts
Copy link
Contributor Author

Okay I've just addressed the comments in this PR. Thanks a lot for the reviews!

@deadlycoconuts deadlycoconuts merged commit e78908c into main Oct 25, 2023
24 of 29 checks passed
@deadlycoconuts deadlycoconuts deleted the add_redeployment_docs branch October 25, 2023 10:13
@deadlycoconuts deadlycoconuts self-assigned this Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants