Skip to content

Conversation

@kannwism
Copy link
Contributor

@kannwism kannwism commented Aug 19, 2024

Uploading images using the generated dataset_api currently only works when generating based on this PR:

OpenAPITools/openapi-generator#19329

@kannwism kannwism marked this pull request as ready for review August 20, 2024 15:27
@kannwism kannwism requested review from LucStr and Makuh17 August 20, 2024 15:27
This requires [openapi-generator](https://github.com/OpenAPITools/openapi-generator) installed

```
brew install openapi-generator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is mainly a macOS thing right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, gotta add instructions for other operating systems at some point, but I think all of us internally would figure it out anyway

This requires [openapi-merge-cli](https://www.npmjs.com/package/openapi-merge-cli) installed globally

```
npm i -g openapi-merge-cli
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we install something needed for the python package with the npm installer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

necessary for the client generation

Comment on lines 34 to +35
question="Who should be president?",
categories=["Kamala Harris", "Donald Trump"],
options=["Kamala Harris", "Donald Trump"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should change this example

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to be in the PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this is the basis for the generated python client, which is why I would track this file

Comment on lines +30 to +36
result = identity_api.identity_get_client_auth_token_post(
client_id=client_id,
_headers={"Authorization": f"Basic {client_secret}"},
)

client_configuration.api_key["bearer"] = f"Bearer {result.auth_token}"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do? These things are not assigned to self, how does that work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it still points to the configuration that was passed to the api_client on initialization, therefore updated it. Would generating a new client be cleaner?

@kannwism kannwism merged commit 1b1d69e into main Aug 22, 2024
@kannwism kannwism deleted the mari/integrate-openapi branch August 22, 2024 14:45
This was referenced Nov 4, 2025
@claude claude bot mentioned this pull request Dec 2, 2025
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

Successfully merging this pull request may close these issues.

3 participants