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

MR Python client get_registered_models inf loop scenario #348

Closed
tarilabs opened this issue Sep 5, 2024 · 0 comments · Fixed by #349
Closed

MR Python client get_registered_models inf loop scenario #348

tarilabs opened this issue Sep 5, 2024 · 0 comments · Fixed by #349
Labels
bug Something isn't working

Comments

@tarilabs
Copy link
Member

tarilabs commented Sep 5, 2024

Describe the bug
infinite loop when registered models are less than default first page

To Reproduce

@pytest.mark.e2e
def test_get_registered_models_no_next_token(client: ModelRegistry):
    models = 3

    for name in [f"test_model{i}" for i in range(models)]:
        client.register_model(
            name,
            "s3",
            model_format_name="test_format",
            model_format_version="test_version",
            version="1.0.0",
        )

    for model in client.get_registered_models():
        print(model)

notice only 3 registered model.

running with

poetry run pytest --e2e -s -k test_get_registered_models_no_next_token

exhibits the follow in the logs:

model-registry  | 2024/09/05 08:17:57 "GET http://localhost:8080/api/model_registry/v1alpha3/registered_models?sortOrder=ASC HTTP/1.1" from 192.168.127.1:33643 - 200 557B in 3.864047ms
id='1' description=None external_id=None create_time_since_epoch='1725524276692' last_update_time_since_epoch='1725524276692' custom_properties=None name='test_model0' owner='author' state=<RegisteredModelState.LIVE: 'LIVE'>
id='3' description=None external_id=None create_time_since_epoch='1725524276870' last_update_time_since_epoch='1725524276870' custom_properties=None name='test_model1' owner='author' state=<RegisteredModelState.LIVE: 'LIVE'>
id='5' description=None external_id=None create_time_since_epoch='1725524276971' last_update_time_since_epoch='1725524276971' custom_properties=None name='test_model2' owner='author' state=<RegisteredModelState.LIVE: 'LIVE'>
model-registry  | 2024/09/05 08:17:57 "GET http://localhost:8080/api/model_registry/v1alpha3/registered_models?sortOrder=ASC&nextPageToken= HTTP/1.1" from 192.168.127.1:33644 - 200 557B in 3.255838ms
id='1' description=None external_id=None create_time_since_epoch='1725524276692' last_update_time_since_epoch='1725524276692' custom_properties=None name='test_model0' owner='author' state=<RegisteredModelState.LIVE: 'LIVE'>
id='3' description=None external_id=None create_time_since_epoch='1725524276870' last_update_time_since_epoch='1725524276870' custom_properties=None name='test_model1' owner='author' state=<RegisteredModelState.LIVE: 'LIVE'>
id='5' description=None external_id=None create_time_since_epoch='1725524276971' last_update_time_since_epoch='1725524276971' custom_properties=None name='test_model2' owner='author' state=<RegisteredModelState.LIVE: 'LIVE'>
model-registry  | 2024/09/05 08:17:57 "GET http://localhost:8080/api/model_registry/v1alpha3/registered_models?sortOrder=ASC&nextPageToken= HTTP/1.1" from 192.168.127.1:33645 - 200 557B in 3.30888ms
id='1' description=None external_id=None create_time_since_epoch='1725524276692' last_update_time_since_epoch='1725524276692' custom_properties=None name='test_model0' owner='author' state=<RegisteredModelState.LIVE: 'LIVE'>
id='3' description=None external_id=None create_time_since_epoch='1725524276870' last_update_time_since_epoch='1725524276870' custom_properties=None name='test_model1' owner='author' state=<RegisteredModelState.LIVE: 'LIVE'>
id='5' description=None external_id=None create_time_since_epoch='1725524276971' last_update_time_since_epoch='1725524276971' custom_properties=None name='test_model2' owner='author' state=<RegisteredModelState.LIVE: 'LIVE'>

... (goes on forever)

Expected behavior
No infinite loops.

Additional context
Please notice when number of entities is above the default page size,
a nextPageToken is passed in the second list-call,
etc,
and empty nextPageToken is passed in the last-call terminating.

ie.

model-registry  | 2024/09/05 08:20:11 "GET http://localhost:8080/api/model_registry/v1alpha3/registered_models?pageSize=10&sortOrder=ASC HTTP/1.1" from 192.168.127.1:33799 - 200 1751B in 4.266298ms
Token changed from None to CBMQExoCCAo at 0
model-registry  | 2024/09/05 08:20:12 "GET http://localhost:8080/api/model_registry/v1alpha3/registered_models?pageSize=10&sortOrder=ASC&nextPageToken=CBMQExoCCAo HTTP/1.1" from 192.168.127.1:33800 - 200 1766B in 3.860088ms
Token changed from CBMQExoCCAo to CCcQJxoCCAo at 10
model-registry  | 2024/09/05 08:20:12 "GET http://localhost:8080/api/model_registry/v1alpha3/registered_models?pageSize=10&sortOrder=ASC&nextPageToken=CCcQJxoCCAo HTTP/1.1" from 192.168.127.1:33801 - 200 224B in 2.989005ms
Token changed from CCcQJxoCCAo to  at 20
model-registry  | 2024/09/05 08:20:12 "GET http://localhost:8080/api/model_registry/v1alpha3/registered_models?pageSize=10&sortOrder=ASC&nextPageToken= HTTP/1.1" from 192.168.127.1:33802 - 200 1751B in 3.520172ms

... and terminates successfully.

this is all I've observed so far.

@tarilabs tarilabs added the bug Something isn't working label Sep 5, 2024
isinyaaa added a commit to isinyaaa/model-registry that referenced this issue Sep 5, 2024
Fixes: kubeflow#348

Signed-off-by: Isabella do Amaral <idoamara@redhat.com>
isinyaaa added a commit to isinyaaa/model-registry that referenced this issue Sep 5, 2024
Fixes: kubeflow#348

Signed-off-by: Isabella do Amaral <idoamara@redhat.com>
google-oss-prow bot pushed a commit that referenced this issue Sep 5, 2024
Fixes: #348

Signed-off-by: Isabella do Amaral <idoamara@redhat.com>
@tarilabs tarilabs added this to the Kubeflow 1.10 roadmap milestone Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant