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

Add engine deprecation note #2856

Merged
merged 9 commits into from
Jan 22, 2021
Merged

Add engine deprecation note #2856

merged 9 commits into from
Jan 22, 2021

Conversation

adriangonz
Copy link
Contributor

What this PR does / why we need it:

Add note to docs about deprecation of Java engine.

Which issue(s) this PR fixes:

Fixes #2840

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Added note on Java engine's deprecation.

@adriangonz
Copy link
Contributor Author

/cc @cliveseldon @RafalSkolasinski

@adriangonz
Copy link
Contributor Author

/test docs

@seldondev
Copy link
Collaborator

Wed Jan 20 16:34:13 UTC 2021
The logs for [lint] [2] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2856/2.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2856 --build=2

@seldondev
Copy link
Collaborator

Wed Jan 20 16:34:23 UTC 2021
The logs for [pr-build] [1] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2856/1.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2856 --build=1

@seldondev
Copy link
Collaborator

Wed Jan 20 16:34:24 UTC 2021
The logs for [docs] [3] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2856/3.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2856 --build=3

RafalSkolasinski
RafalSkolasinski suggested changes Jan 20, 2021
View reviewed changes
Copy link
Contributor

@RafalSkolasinski RafalSkolasinski 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, added few comments

doc/source/graph/svcorch.md Outdated
The current default orchestrator in Go the "executor" does not return routing meta data in request calls. This is a [known issue](https://github.com/SeldonIO/seldon-core/issues/1823).
Copy link
Contributor

Choose a reason for hiding this comment

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

This was re-added recently but requires environmental variable to be set. Worth noting it here.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the PR that added it again: #2723

Copy link
Contributor

Choose a reason for hiding this comment

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

Just don't remember what was the environmental variable to set...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot @RafalSkolasinski ! Should I just remove that line?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would definitely make sure that this is documented somewhere around the orchestrator in a visible place as it was one of the blockers for ppl from moving to executor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it's no longer an issue, wouldn't it be better to remove it completely? Thinking of 2 or 3 versions ahead, wouldn't it be confusing for users to see a specific section for something that works?

Or is it the case that users require some guidance to use the routing metadata?

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've now added a note on the routing metadata injection env var @RafalSkolasinski. Would be great to hear your thoughts!

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks perfect, nice one 👍

doc/source/graph/svcorch.md
since Seldon Core `1.1`.

For further details on the Java engine see previous versions of this page in the docs.
Copy link
Contributor

Choose a reason for hiding this comment

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

Worth adding direct link to previous release docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To which one though? I'd leave it up to the user to choose the right version for their docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say last one that contained it. Though it's not mandatory.

@adriangonz
Copy link
Contributor Author

/test docs

@seldondev
Copy link
Collaborator

Thu Jan 21 11:37:10 UTC 2021
The logs for [pr-build] [4] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2856/4.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2856 --build=4

@seldondev
Copy link
Collaborator

Thu Jan 21 11:38:03 UTC 2021
The logs for [lint] [5] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2856/5.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2856 --build=5

@seldondev
Copy link
Collaborator

Thu Jan 21 11:38:15 UTC 2021
The logs for [docs] [6] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2856/6.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2856 --build=6

@adriangonz
Copy link
Contributor Author

/test docs

@seldondev
Copy link
Collaborator

Thu Jan 21 16:02:58 UTC 2021
The logs for [pr-build] [7] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2856/7.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2856 --build=7

@seldondev
Copy link
Collaborator

Thu Jan 21 16:03:09 UTC 2021
The logs for [docs] [9] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2856/9.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2856 --build=9

@seldondev
Copy link
Collaborator

Thu Jan 21 16:03:18 UTC 2021
The logs for [lint] [8] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2856/8.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2856 --build=8

@axsaucedo
Copy link
Contributor

Lookks good @RafalSkolasinski good to go?

@RafalSkolasinski RafalSkolasinski self-requested a review January 22, 2021 13:52
@seldondev seldondev added the lgtm label Jan 22, 2021
@seldondev
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RafalSkolasinski

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@seldondev
Copy link
Collaborator

Fri Jan 22 13:56:21 UTC 2021
The logs for [lint] [11] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2856/11.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2856 --build=11

@seldondev
Copy link
Collaborator

Fri Jan 22 13:56:29 UTC 2021
The logs for [pr-build] [10] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2856/10.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2856 --build=10

@seldondev seldondev merged commit ef3b2fc into SeldonIO:master Jan 22, 2021
@adriangonz adriangonz deleted the add-deprecation-note branch January 22, 2021 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RafalSkolasinski RafalSkolasinski RafalSkolasinski approved these changes

@ukclivecox ukclivecox Awaiting requested review from ukclivecox

Assignees

@RafalSkolasinski RafalSkolasinski

Labels
approved lgtm size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update engine docs as deprecated
4 participants
@adriangonz @seldondev @axsaucedo @RafalSkolasinski