Hyperfoil / horreum-client-python

Python library and examples for Horreum
Apache License 2.0
0 stars 2 forks source link

Create a first PoC #1

Closed stalep closed 4 months ago

stalep commented 10 months ago

Using https://horreum.hyperfoil.io/openapi/ it should be possible to create a Python client connecting to Horreum. There might be obstacles we do not know about, so a PoC would be welcome To get started: Download the API:

wget https://horreum.hyperfoil.io/openapi/openapi.yaml
pip install openapi-python-client
openapi-python-client generate --path openapi.yaml
webbnh commented 9 months ago

Hi @stalep, I'm gearing up to try to produce this. (I'm working my way through the setup instructions -- it's been an odyssey -- I'm up to trying to run the Horreum tests, but they are failing due to a lack of Docker...so that's the next stumbling block for me to remove.)

If you want to assign this issue to me, feel free.

stalep commented 8 months ago

Hi @webbnh, we've noticed an issue with the openapi-python generator in the latest version of Horreum. I'm looking at it and we plan to have it fixed in the upcoming version. I'll ping you when it's fixed in master!

stalep commented 8 months ago

They openapi-python generator now work with master @webbnh

webbnh commented 7 months ago

Hi all,

Contrary to all appearances, I am trying to make headway on this stuff. 🥴

I was able to generate the API client, but the tool has some complaints, and so it doesn't generate code for all the APIs:

$ openapi-python-client generate --path openapi.yaml
Generating horreum-rest-api-client
Warning(s) encountered while generating. Client was generated, but some pieces may be missing

WARNING parsing POST /api/run/{id}/description within run. Endpoint will not be generated.

Unsupported content type text/plain

WARNING parsing POST /api/run/{id}/schema within run. Endpoint will not be generated.

Unsupported content type text/plain

I got confirmation from the tool's maintainer: it indeed does not currently support text/plain content for request bodies.

We have various options. The simplest might be to tweak the YAML for those two APIs:

  /api/run/{id}/description:
    post:
      [...]
      requestBody:
        content:
          text/plain:
            schema:
              type: string
      [...]

and replace text/plain with application/octet-stream, which is a supported type. I don't think that doing so will have any negative effects, and it allows the tool to generate those two APIs successfully.

I'm not blocked by this, but let me know if you want me to post a PR with this change.

stalep commented 7 months ago

Hi @webbnh we have a pr open that fixes the python generation. Hoping to have it merged by tomorrow!

webbnh commented 7 months ago

Hi @stalep, what was the PR, and has it been merged, yet? (Was it https://github.com/Hyperfoil/Horreum/pull/1292?)

The reason I ask is that, at JoeT's prompting, I've found a bug in the openapi.yaml file: the /components/schemas/ProtectedTimeType schema says that the start field is a date-time string (like 2019-09-26T07:58:30.996+0200), but the API response contains 'start': 1697154107508. The stop field looks to have the same issue.

Do you want me to create a PR?

Here's the change:

diff --git a/docs/site/content/en/openapi/openapi.yaml b/docs/site/content/en/openapi/openapi.yaml
index 95085210..ecac9851 100644
--- a/docs/site/content/en/openapi/openapi.yaml
+++ b/docs/site/content/en/openapi/openapi.yaml
@@ -3240,15 +3240,15 @@ components:
           type: string
           example: performance-team
         start:
-          format: date-time
+          format: int64
           description: Run Start timestamp
-          type: string
-          example: 2019-09-26T07:58:30.996+0200
+          type: integer
+          example: 1698013206000
         stop:
-          format: date-time
+          format: int64
           description: Run Stop timestamp
-          type: string
-          example: 2019-09-26T07:58:30.996+0200
+          type: integer
+          example: 1698013206000
     ProtectedType:
       required:
       - access
willr3 commented 6 months ago

@webbnh openapi.yaml is generated from code annotations on api classes so unfortunately changes to the file will get replaced each time the project compiles. I am also working on the compatibility with openapi-python-client. I wish they would add support for text/plain content type :) https://github.com/openapi-generators/openapi-python-client/discussions/821

The example on ProtectedTimeType is something we can easily change but I suspect the example is based on the data format for adding data into Horreum. Do you mind sharing the endpoint you queried when you got the unexpected start and stop? (or at least the /api/... portion of the endpoint)

willr3 commented 6 months ago

I opened an issue in the project that creates the openapi.yaml https://github.com/Hyperfoil/Horreum/issues/1336

webbnh commented 6 months ago

openapi.yaml is generated from code annotations on api classes so unfortunately changes to the file will get replaced each time the project compiles.

@willr3, thanks for mentioning, that. BTW, what do you mean by "each time the project compiles"?...it's a little weird to have a build output checked into the repository....

I am also working on the compatibility with openapi-python-client.

Ah, good...so I'm not alone! 😁

The example on ProtectedTimeType is something we can easily change but I suspect the example is based on the data format for adding data into Horreum. Do you mind sharing the endpoint you queried when you got the unexpected start and stop? (or at least the /api/... portion of the endpoint)

The field at issue is either format or type...I just changed example to keep the three fields consistent.

The first API I hit the problem in is /api/dataset/list/{testId}, but there are others which use the ProtectedTimeType, as well.

willr3 commented 6 months ago

I opened a PR that fix the annotations and should generate an openapi.yaml that properly describes start and stop as Long. https://github.com/Hyperfoil/Horreum/pull/1337 I will be away for a week but we have a PR open to address the other type issues with openapi-python-client https://github.com/Hyperfoil/Horreum/pull/1335 hopefully during that week we get some news on text/plain https://github.com/openapi-generators/openapi-python-client/discussions/821

webbnh commented 6 months ago

I opened a PR that fix the annotations and should generate an openapi.yaml that properly describes start and stop as Long [and] we have a PR open to address the other type issues with openapi-python-client

💪

hopefully during that week we get some news on text/plain openapi-generators/openapi-python-client#821

I admire your optimism. (Did you see https://github.com/openapi-generators/openapi-python-client/issues/699?)

willr3 commented 6 months ago

my optimism is waning given: 699

How are you testing the python client? Do you start a new project and import the generated horreum-rest-api-client folder? If you could give an example of what that looks like in python (like you would give to a kid who is just learning python) I would like to start testing the generated client when I get back in a week.

webbnh commented 6 months ago

I went with the simplistic approach for my first pass:

My current script is just endeavoring to try out all of the APIs that I can. It doesn't do much that is actually useful, other than to call them and report the result. So far, I'm only trying the GET methods and only the methods which don't require authentication. So, it's more of a functional test than a PoC at this point. Once I've gotten everything shaken down, I'll move on to create another script which is more useful. But, as a demonstration of what one might do with the client, it's a fair start.

Here's what I've got so far. ```python #! /usr/bin/env python3 from horreum_rest_api_client import AuthenticatedClient, Client from horreum_rest_api_client.api.config.version import sync_detailed as version from horreum_rest_api_client.api.config.keycloak import sync_detailed as keycloak from horreum_rest_api_client.api.config.test_datastore import sync_detailed as datastore_test from horreum_rest_api_client.api.dataset.get_dataset import sync_detailed as get_dataset from horreum_rest_api_client.api.dataset.get_summary import sync_detailed as dataset_summary from horreum_rest_api_client.api.dataset.label_values import sync_detailed as dataset_label_values from horreum_rest_api_client.api.dataset.list_by_test import sync_detailed as dataset_list_test from horreum_rest_api_client.api.dataset.list_by_schema import sync_detailed as dataset_list_schema from horreum_rest_api_client.api.experiment.models import sync_detailed as models from horreum_rest_api_client.api.experiment.profiles import sync_detailed as profiles from horreum_rest_api_client.api.experiment.run_experiments import sync_detailed as exp_run from horreum_rest_api_client.api.run.list_all_runs import sync_detailed as run_list from horreum_rest_api_client.api.schema.list_ import sync_detailed as schema_list from horreum_rest_api_client.api.test.folders import sync_detailed as test_folders from horreum_rest_api_client.api.test.list_ import sync_detailed as test_list production_instance = "https://horreum.corp.redhat.com" local_instance = "http://localhost:8080/" output_width = 140 def api_call(api_str, fn, **kwargs): print(f"{api_str}:", end="\t") response = fn(client=client, **kwargs) if 200 <= response.status_code < 300: output = str(response.parsed)[:output_width] if len(output) >= output_width: output += "..." print(output) else: print("failed with", response.status_code) return response a_client = AuthenticatedClient( base_url=production_instance, raise_on_unexpected_status=True, token="secret", # I don't think this is being checked with the test server verify_ssl=False, ) u_client = Client(base_url=production_instance, verify_ssl=False) with u_client as client: api_call("version", version) api_call("keycloak", keycloak) api_call("datastore_test", datastore_test, id=294) # Requires authorization api_call("test list", test_list) api_call("test folders", test_folders, roles="__all") api_call("schema list", schema_list) api_call("run list", run_list) api_call("ds list schema", dataset_list_schema, uri="urn:ckq_memory_cpu2:0.1") api_call("ds list test", dataset_list_test, test_id=294) api_call("ds label vals", dataset_label_values, dataset_id=124811) api_call("ds summary", dataset_summary, dataset_id=124811) api_call("get dataset", get_dataset, id=124811) api_call("exp profiles", profiles, test_id=294) # need a test which has a profile # api_call("run experiment", exp_run, dataset_id=124810) # crashes with a dataset which has no profiles ```

In "real" code, you presumably wouldn't use my api_call() wrapper, but it handles the repetitive boiler plate for me. The import magic makes the resulting code more expressive:

    with u_client as client:
        response = version(client=client)
        response = get_dataset(client=client, id=124811)
        // etc.
willr3 commented 6 months ago

Thank you for the code. I was able to get more of it running with the 'openapi.yaml' changes in my PR than with master so we are taking steps in the right direction

You asked earlier what I mean about generating the openapi.yaml. The horreum-api module of the Horreum project has code annotations on the service endpoints and the data classes. Part of the compilation creates a new openapi.yaml and places it in the docs module. The same compilation then uses the openapi.yaml to generate the Typescript client for the web interface module in Horreum. We have caught breaking changes to data types or endpoint paths with this method but nothing is perfect.

webbnh commented 6 months ago

Thank you for the code. I was able to get more of it running with the 'openapi.yaml' changes in my PR than with master so we are taking steps in the right direction

Excellent. I've been accumulating fixes here, although I'm a few commits behind master, now.

You asked earlier what I mean about generating the openapi.yaml. The horreum-api module of the Horreum project has code annotations on the service endpoints and the data classes. Part of the compilation creates a new openapi.yaml and places it in the docs module. The same compilation then uses the openapi.yaml to generate the Typescript client for the web interface module in Horreum. We have caught breaking changes to data types or endpoint paths with this method but nothing is perfect.

All of that makes good sense, except for the part where the YAML file is supposed to be committed to the repo before the build which is supposed to create it. 😆

I see that John has made some changes to the Maven configuration which look relevant (unfortunately, I don't speak Maven), but, really, the openapi.yaml file should be a build output (like a .jar file or documentation or anything else generated by the build), and it should be handled similarly, not like a source file pulled from the repo: then the interface spec can never be out of sync with the interface, because they are built from the same source at the same time.

webbnh commented 6 months ago

I've been accumulating fixes here, although I'm a few commits behind master, now.

I've resync'd with master, and it looks like you guys have caught up to me, except for the text/plain thing. 😉

And, yes, everything that I have in the demo script so far seems to work. 🎉

johnaohara commented 6 months ago

All of that makes good sense, except for the part where the YAML file is supposed to be committed to the repo before the build which is supposed to create it. 😆

We host the src code and the documentation for the website (https://horreum.hyperfoil.io/) in the same repo. This way we can ensure that PRs with functional changes contain all user documentation and the website is always up-to-date with the latest stable branch of Horrum.

There is a bit of an anomaly with this set-up, in that, for build artefacts that are published on the website (which is only openapi.yaml currently) , we have to ensure that the changes to openapi.yaml are checked in with the PR.

The yaml is generated as part of the build, and after a change has been made and tested locally the src code change and the openapi.yaml changes should be checked in the same PR.

I see that John has made some changes to the Maven configuration which look relevant (unfortunately, I don't speak Maven), but, really, the openapi.yaml file should be a build output (like a .jar file or documentation or anything else generated by the build), and it should be handled similarly, not like a source file pulled from the repo: then the interface spec can never be out of sync with the interface, because they are built from the same source at the same time.

There was an issue with maven not running certain phases given some conditions, the recent changes rectify this. the openapi.yaml is a build output, and is used to automatically build the Tyepscript client, but atm we do not automatically build python/go clients (we have talked about it but have not opened an issue).

The main issue I see with treating openapi.yaml as a build artefact is we would have to go through a full Horreum release to consume it in downstream projects, but when we are developing clients (and changing the api spec) we do not want a published version for pple to consume, but we want to consume from the master branch which contains ongoing fixes. We also do not want to publish an unstable openapi.yaml on the website, we wish to keep that in sync with the latest stable Horreum release

webbnh commented 6 months ago

Thanks, @johnaohara. While you were typing that in here, I was preaching over here. 😀

Even so, it might be good if you had a test which verified that the openapi.yaml file produced by the build matched the one from the docs tree in the source repo.

johnaohara commented 6 months ago

Even so, it might be good if you had a test which verified that the openapi.yaml file produced by the build matched the one from the docs tree in the source repo.

I think that is a good idea :) atm it is a manual check.

The anomaly exists because we have taken a shortcut in hosting the website artefacts in the same repo as the src code. we should update the the website build to pull in an openapi.yaml that has been published somewhere as part of a release.

We have another concern in that for developers developing against the current master branch we probably should publish the openapi.yaml somewhere without having to expect them to build the project from a snapshot. i.e. if you are a python dev, working on a python client, we wouldn't expect you to install the entire java toolchain just to get the current SNAPSHOT openapi.yaml. We have chosen to publish it in the master branch, although we could update the build process to publish it is a SNAPSHOT openapi.yaml on the website or in github

stalep commented 6 months ago

Hi @webbnh, I just ran the openapi python gen client from master and it ran without any warnings/errors.

stalep commented 6 months ago

We now have a pr (https://github.com/Hyperfoil/Horreum/pull/1360) that will generate python and go code as part of the Horreum build.

webbnh commented 6 months ago
I've got my little demo script converted to the new Python client. #! /usr/bin/env python3 from openapi_client.api_client import ApiClient from openapi_client.configuration import Configuration from openapi_client.api.config_api import ConfigApi from openapi_client.api.dataset_api import DatasetApi from openapi_client.api.experiment_api import ExperimentApi from openapi_client.api.run_api import RunApi from openapi_client.api.schema_api import SchemaApi from openapi_client.api.test_api import TestApi production_instance = "https://horreum.corp.redhat.com" local_instance = "http://localhost:8080/" output_width = 158 def api_call(sect, fn, *args, **kwargs): pref = f"{sect}-{fn.__name__}:" pref = f"{pref:24.24}" try: response = fn(*args, **kwargs) except Exception as e: response = " ".join(f"Failed with {e}".splitlines()) output = pref + str(response) if len(output) >= output_width: output = output[:output_width - 3] + "..." print(output) with ApiClient(configuration=Configuration(host=production_instance)) as c: c.set_default(c) api_call("config", ConfigApi().version) api_call("config", ConfigApi().keycloak) api_call("config", ConfigApi().datastores, team="294") api_call("config", ConfigApi().test_datastore, id="294") api_call("test", TestApi().list) api_call("test", TestApi().get, id=294) api_call("test", TestApi().folders, roles="__all") api_call("schema", SchemaApi().list) api_call("run", RunApi().list_all_runs) api_call("dataset", DatasetApi().list_by_schema, uri="urn:ckq_memory_cpu2:0.1") api_call("dataset", DatasetApi().list_by_test, test_id=294) api_call("dataset", DatasetApi().label_values, dataset_id=124811) api_call("dataset", DatasetApi().get_summary, dataset_id=124811) api_call("dataset", DatasetApi().get_dataset, id=124811) api_call("experiment", ExperimentApi().profiles, test_id=294) # need a test which has a profile api_call("experiment", ExperimentApi().run_experiments, dataset_id=124810)

Unfortunately, almost all of the responses fail to parse. I don't know whether that's a problem in what I'm doing, or a problem in the openapi.yaml file, or in the generated code, or something else. I'll try to work on this some more tomorrow.

willr3 commented 6 months ago

I want to create an example python project to run through the api with a test DB when I get back from vacation. I mostly use python for Jupiter notebooks. @webbnh is there a python unit test library you would recommend for creating the tests for the python client?

webbnh commented 6 months ago

is there a python unit test library

@willr3, I'm accustomed to using PyTest and to running it under Tox.

However, it's not obvious to me that you want to write unit tests for this. (It could be a terminology problem, but unit tests generally operate by isolating certain portions of the code-under-test (aka CUT) from their dependencies, by replacing the dependencies with fake or mock implementations of them.) What I think you want to test is integration -- that the generated client actually works with the real API implementation -- and for that you'll need functional tests.

This is not to say that PyTest cannot be used to implement functional testing (it can, but its forte is unit testing). But, I just want to make sure that your focus is useful and that your choice of words doesn't confuse things. 🙂

jtaleric commented 6 months ago

Coming out of PerfConf I am excited to see the fruit of this investigation!

In chatting with @johnaohara in RDU I wanted to put a comment here about shipping a python client package versus expecting users to generate the client from OpenAPI. It is my understanding with chatting with @johnaohara the expectation is there will be a python client shipped with each version of horreum -- there isn't an expectation the user builds the client from the OpenAPI spec.

webbnh commented 6 months ago

@jtaleric, that is my understanding, as well: the Horreum build now produces a Python client. This needs to be packaged as a Python "package" and made available for consumers' use. So, there are two work items (beyond making sure it all works, which is what I'm currently exploring, albeit at a glacial pace): get the Horreum build to produce the package file(s) [I'm sure this is well-trodden ground, but I myself haven't had to tread it, yet], and devise a policy and mechanism for publishing them [doing "releases" is reasonably straightforward; however, making branch builds, etc. available might require more "stuff"].

jtaleric commented 6 months ago

@jtaleric, that is my understanding, as well: the Horreum build now produces a Python client.

ack. I know there has been some discussions with shipping a client package vs users building a client, I just wanted to make sure everyone is on the same page, and it sounds like we are so great! 😄

webbnh commented 5 months ago

Presumably this should be assigned to @lampajr for closure. 🙂

lampajr commented 4 months ago

Presumably this should be assigned to @lampajr for closure. 🙂

Sorry I missed this comment :disappointed:

Closing this issue :rocket: