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.

Sign up for GitHub

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

Jump to bottom

custom server requirements.txt file generation added to the notebook #711

Merged
merged 3 commits into from
Sep 5, 2022
Merged

custom server requirements.txt file generation added to the notebook #711

merged 3 commits into from
Sep 5, 2022

Conversation

saeid93
Copy link
Contributor

@saeid93 saeid93 commented Sep 1, 2022

Fixes #710

adriangonz
adriangonz suggested changes Sep 2, 2022
View reviewed changes
Copy link
Contributor

@adriangonz adriangonz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice one @saeid93! Thanks a lot for adding this one.

Just added a small comment below about the file name. Once that's addressed, I think this should be good to go! 👍

PS: Almost forgot! Once you're done, could you run the make -C docs/examples all command to ensure that the equivalent markdown is also generated?

docs/examples/custom/README.ipynb Outdated
"metadata": {},
"outputs": [],
"source": [
"%%writefile models.py\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this line be %%writefile requirements.txt instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, That's why I shouldn't do all the pull requests late at night. :D
Should work now. Please check.

adriangonz
adriangonz approved these changes Sep 5, 2022
View reviewed changes
Copy link
Contributor

@adriangonz adriangonz left a comment

Choose a reason for hiding this comment

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

Nice one! Thanks for adding the changes.

Once the tests are green, this should be good to go 👍

@adriangonz adriangonz merged commit f4ba2b0 into SeldonIO:master Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adriangonz adriangonz adriangonz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

missing mentioning the requirements.txt in the "serving custom model page"
2 participants
@saeid93 @adriangonz