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

include rclone in adserver #3172

Merged
merged 16 commits into from
May 14, 2021
Merged

include rclone in adserver #3172

merged 16 commits into from
May 14, 2021

Conversation

RafalSkolasinski
Copy link
Contributor

@RafalSkolasinski RafalSkolasinski commented May 7, 2021 •
edited
Loading

This extends seldonio/alibi-detect-server to include rclone as possible artifact dowload solution.

If RCLONE_ENABLED=true environmental variable is set or any of the enviromental variables contain RCLONE_CONFIG in their name then rclone will be used to download the artefacts. If RCLONE_ENABLED=false or no RCLONE_CONFIG variables are present then kfserving storage.py logic will be used to download the artefacts.

Closes #3189

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@RafalSkolasinski RafalSkolasinski marked this pull request as draft May 7, 2021 19:05
@seldondev seldondev added size/XL and removed size/L labels May 12, 2021
@RafalSkolasinski
Copy link
Contributor Author

/test integration

1 similar comment
@RafalSkolasinski
Copy link
Contributor Author

/test integration

@RafalSkolasinski RafalSkolasinski marked this pull request as ready for review May 12, 2021 17:35
@RafalSkolasinski RafalSkolasinski changed the title wip: include rclone in adserver include rclone in adserver May 12, 2021
@RafalSkolasinski
Copy link
Contributor Author

The two new integration tests seems to failed. Trying to set up locally exactly the setup used in CI to see what goes wrong

@axsaucedo
Copy link
Contributor

/test integration

4 similar comments
@axsaucedo
Copy link
Contributor

/test integration

@axsaucedo
Copy link
Contributor

/test integration

@axsaucedo
Copy link
Contributor

/test integration

@axsaucedo
Copy link
Contributor

/test integration

@axsaucedo
Copy link
Contributor

/test notebooks

@axsaucedo
Copy link
Contributor

/test integration

4 similar comments
@RafalSkolasinski
Copy link
Contributor Author

/test integration

@axsaucedo
Copy link
Contributor

/test integration

@axsaucedo
Copy link
Contributor

/test integration

@axsaucedo
Copy link
Contributor

/test integration

@axsaucedo
Copy link
Contributor

/test notebooks

4 similar comments
@axsaucedo
Copy link
Contributor

/test notebooks

@axsaucedo
Copy link
Contributor

/test notebooks

@axsaucedo
Copy link
Contributor

/test notebooks

@axsaucedo
Copy link
Contributor

/test notebooks

@axsaucedo
Copy link
Contributor

/test integration

1 similar comment
@axsaucedo
Copy link
Contributor

/test integration

@axsaucedo
Copy link
Contributor

/test integration

4 similar comments
@axsaucedo
Copy link
Contributor

/test integration

@RafalSkolasinski
Copy link
Contributor Author

/test integration

@RafalSkolasinski
Copy link
Contributor Author

/test integration

@RafalSkolasinski
Copy link
Contributor Author

/test integration

@seldondev
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign rafalskolasinski
You can assign the PR to them by writing /assign @rafalskolasinski in a comment when ready.

The full list of commands accepted by this bot can be found 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

@RafalSkolasinski
Copy link
Contributor Author

/test notebooks

@RafalSkolasinski
Copy link
Contributor Author

image

@RafalSkolasinski
Copy link
Contributor Author

And integration also passed
image

@ukclivecox ukclivecox merged commit 1dfd8d7 into SeldonIO:master May 14, 2021
@RafalSkolasinski RafalSkolasinski mentioned this pull request Jun 24, 2021
Closed
@RafalSkolasinski
Copy link
Contributor Author

Includes test for #2681

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Adjust outlier examples to use rclone based storage initializer
4 participants
@RafalSkolasinski @axsaucedo @seldondev @ukclivecox