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

ambassador v1 api #492

Merged
merged 4 commits into from
Apr 8, 2019
Merged

ambassador v1 api #492

merged 4 commits into from
Apr 8, 2019

Conversation

ryandawsonuk
Copy link
Contributor

fixes #491

I checked against the specs referenced in that issue and did a step through of the timeouts notebook to check that feature works.

I also ran the E2E tests - I did get a failure but it looks like an instance of #473 Surprised to see that even with the retry library there though

@ryandawsonuk
Copy link
Contributor Author

ryandawsonuk commented Apr 4, 2019

Actually when I run pytest -s -W ignore test_helm_charts_single_namespace.py it really does fail consistently

@ryandawsonuk ryandawsonuk changed the title ambassador v1 api WIP: ambassador v1 api Apr 4, 2019
@ryandawsonuk ryandawsonuk changed the title WIP: ambassador v1 api ambassador v1 api Apr 4, 2019
@ryandawsonuk
Copy link
Contributor Author

With 561157c it now passes for me.

I was able to recreate the behaviour in a notebook, though it does not happen every time so was not easy to reproduce. Seems like it's not just that the first grpc request after deployment fails, it's that sometimes it can take minutes for the grpc to start working, even after rollout reports completion (which is longer than the retries were allowing for)

@seldondev seldondev added size/M and removed size/S labels Apr 5, 2019
@ryandawsonuk
Copy link
Contributor Author

ryandawsonuk commented Apr 5, 2019

After discussion on the ambassador slack and the linked ambassador issue, have found we can get the grpc requests working in a timely fashion by running ambassador as root so have updated PR to do that

@ukclivecox ukclivecox merged commit 5f4d0d6 into master Apr 8, 2019
@ryandawsonuk ryandawsonuk deleted the 491-ambassador-v1api branch April 8, 2019 14:50
@ryandawsonuk
Copy link
Contributor Author

Turns out grpc requests are better but still intermittent problems - #498

@ryandawsonuk ryandawsonuk mentioned this pull request May 30, 2019
agrski pushed a commit that referenced this pull request Dec 2, 2022
* Fix strimzi helm values bug for zookeeper persistent storage

* fix kafka bootstrap location to be internal SVC
agrski pushed a commit that referenced this pull request Dec 2, 2022
* fix small typo

* do not set default min replicas

* consider model loaded on replica when filtering

* add unit test

* add more tests

* add very simple crude model scaling logic

* refactor function

* lint fixes

* lint fixes

* add test

* reorder funcs

* add smoke test

* fix func name

* add scaling down test

* refactor

* lint

* simplify test

* set defaults if min / max replicas are missing

* add range checks

* tidy up scaling range logic

* more tidy up

* add more tests

* add cooling off period

* fix lint

* add range checks for replicas

* remove unused env variables

* remove  lock

* revert REQUESTS_CA_BUNDLE (#491)

* tidy up notebook with more models for triton (#494)

* Fix Strimzi Helm values ZK indent bug + stale broker service name (#492)

* Fix strimzi helm values bug for zookeeper persistent storage

* fix kafka bootstrap location to be internal SVC

* fix log message

* add env variables to runner

* tidy up comment

* switch log to debug

* do not update k8s meta if nil

* tidy up ,model name

* fix decrement bug

Co-authored-by: cliveseldon <cc@seldon.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

use v1 ambassador api
3 participants