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

Create a first PoC #1

Closed
stalep opened this issue Oct 29, 2023 · 29 comments
Closed

Create a first PoC #1

stalep opened this issue Oct 29, 2023 · 29 comments
Assignees

Comments

@stalep
Copy link
Member

stalep commented Oct 29, 2023

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
Copy link

webbnh commented Dec 13, 2023

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
Copy link
Member Author

stalep commented Jan 3, 2024

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
Copy link
Member Author

stalep commented Jan 10, 2024

They openapi-python generator now work with master @webbnh

@webbnh
Copy link

webbnh commented Feb 5, 2024

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
Copy link
Member Author

stalep commented Feb 7, 2024

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

@webbnh
Copy link

webbnh commented Feb 16, 2024

Hi @stalep, what was the PR, and has it been merged, yet? (Was it Hyperfoil/Horreum#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
Copy link

willr3 commented Feb 19, 2024

@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 :)
openapi-generators/openapi-python-client#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
Copy link

willr3 commented Feb 19, 2024

I opened an issue in the project that creates the openapi.yaml
Hyperfoil/Horreum#1336

@webbnh
Copy link

webbnh commented Feb 19, 2024

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
Copy link

willr3 commented Feb 19, 2024

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

@webbnh
Copy link

webbnh commented Feb 19, 2024

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 openapi-generators/openapi-python-client#699?)

@willr3
Copy link

willr3 commented Feb 19, 2024

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
Copy link

webbnh commented Feb 19, 2024

I went with the simplistic approach for my first pass:

  • created a fresh directory and dropped the openapi.yaml file into it
  • ran openapi-python-client generate --path openapi.yaml
  • cd'd to the resulting horreum-rest-api-client directory
  • created a demo.py script, there (so that the Python path works out nicely by default)
  • With that as my current directory (and with the x bit set on the script file), I just run ./demo.py

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.
#! /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
Copy link

willr3 commented Feb 20, 2024

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
Copy link

webbnh commented Feb 20, 2024

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
Copy link

webbnh commented Feb 20, 2024

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
Copy link
Member

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
Copy link

webbnh commented Feb 20, 2024

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
Copy link
Member

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
Copy link
Member Author

stalep commented Feb 21, 2024

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

@stalep
Copy link
Member Author

stalep commented Feb 21, 2024

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

@webbnh
Copy link

webbnh commented Feb 21, 2024

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
Copy link

willr3 commented Feb 23, 2024

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
Copy link

webbnh commented Feb 26, 2024

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
Copy link

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
Copy link

webbnh commented Mar 18, 2024

@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
Copy link

@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 webbnh removed their assignment Apr 3, 2024
@webbnh
Copy link

webbnh commented Apr 3, 2024

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

@lampajr lampajr self-assigned this Apr 25, 2024
@lampajr
Copy link
Member

lampajr commented Apr 25, 2024

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

Sorry I missed this comment 😞

Closing this issue 🚀

@lampajr lampajr closed this as completed Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants