8000 eng: Update code to be editable BNCH-125859 by peter-goldstein · Pull Request #229 · benchling/openapi-python-client · GitHub
[go: up one dir, main page]

Skip to content

eng: Update code to be editable BNCH-125859 #229

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

Merged
merged 6 commits into from
May 24, 2025

Conversation

peter-goldstein
Copy link
@peter-goldstein peter-goldstein commented May 22, 2025

Fix the prod/1.x branch so it's possible to run the client library generator from a local clone.

Changes to make it work at all:

pyproject.toml

  • We'll use python 3.9 because 3.7 is dead-dead
  • PyYAML <6 isn't installable anymore, thanks to PEP 517
  • shellingham 1.5.0 was yanked, so we need a later version
  • Include type annotations for PyYAML for good measure

Changes to make the generated output match what's already published:

poetry.lock

  • Downgrade black so it tolerates leading and trailing spaces in triple- quoted strings
  • Upgrade autoflake to match the version in aurelia
  • Upgrade isort to match the version in aurelia

openapi_python_client/templates/pyproject.toml

  • Enforce line length of 110 characters
  • Configure isort to use the same settings as in aurelia (don't separate by "from foo import" vs "import"; don't separate constants-style imports from other imports)

openapi_python_client/__init__.py

Testing

The goal was to make it possible to iterate on the python client generator in-repo. We will have succeeded if we can run the generator on the spec in Aurelia and get the same files that are in the released benchling-api-client. I have downloaded the latest published package (benchling_api_client-2.0.387) and will use it for diffing.

git clone git@github.com:benchling/openapi-python-client.git
cd openapi-python-client
git switch prod/1.x
pyenv local 3.9
poetry env use 3.9
poetry install
poetry run openapi-python-client generate \
  --path=$HOME/benchling/aurelia/benchling/dev_platform/specs/__dist/la/api/v2/openapi.yaml \
  --custom-template-path=$HOME/benchling/aurelia/packages/api_client_generation/client_gen/templates
diff -rq \
  ./benchling-api-client/benchling_api_client \
  $HOME/Downloads/benchling_api_client-2.0.387\ 2/benchling_api_client/v2/stable

The resulting list of files that differ is a subset of the files that Aurelia's packages/api_client_generation/client_gen/create-client.py overrides

  • :121

    • > openapi.yaml
  • :184

    • ≠ models/__init__.py
  • :185

    • ≠ __init__.py
    • ≠ client.py
    • > extensions.py
    • < py.typed
    • ≠ types.py

pyproject.toml
- We'll use python 3.9 because 3.7 is dead-dead
- PyYAML <6 isn't installable anymore, thanks to PEP 517
- shellingham 1.5.0 was yanked, so we need a later version
- Include type annotations for PyYAML for good measure

poetry.lock
- Downgrade black so it tolerates leading and trailing spaces in triple-
  quoted strings
- Upgrade autoflake to match the version in aurelia
- Upgrade isort to match the version in aurelia

openapi_python_client/templates/pyproject.toml
- Enforce line length of 110 characters
- Configure isort to use the same settings as in aurelia (don't separate
  by "from foo import" vs "import"; don't separate constants-style
  imports from other imports)

openapi_python_client/__init__.py
- Run autoflake from outside the generated client package. This is
  necessary because autoflake will crash if it runs against a file
  called types.py. We have a file called types.py.
@peter-goldstein peter-goldstein marked this pull request as ready for review May 22, 2025 02:48
Comment on lines +61 to +64
# TODO(BNCH-125868) Re-add safety check
# && safety check --bare\
# TODO(BNCH-125869) Re-add mypy check
# && mypy openapi_python_client\

Choose a reason for hiding this comment

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

OOC why did we need to remove these two things for now? are they incompatible with some of the other changes in the PR?

Copy link
Author

Choose a reason for hiding this comment

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

Nope, they're broken at head. I fixed what I could without going down a very deep rabbit hole. For the others, I created new tickets and added TODOs here.

Choose a reason for hiding this comment

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

gotcha, I'm totally fine w that approach

pyproject.toml Outdated
jinja2 = "^3.1.0"
stringcase = "^1.2.0"
typer = "^0.3"
colorama = {version = "^0.4.3", markers = "sys_platform == 'win32'"}
shellingham = "^1.3.2"
shellingham = "^1.5.4"
black = ">=20.8b1"

Choose a reason for hiding this comment

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

this line in your PR description mentions changes made in poetry.lock

Downgrade black so it tolerates leading and trailing spaces in triple- quoted strings

I am likely misunderstanding something about poetry, but were the poetry.lock black changes made by hand? if not, how did black get downgraded to "20.8b1" in the lockfile without making any changes here - i.e. do we need to pin the version here so that we don't unintentionally re-upgrade it in the future?

Choose a reason for hiding this comment

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

same question applies for the other packages mentioned as having their versions changed in the lockfile, autoflake and isort

Copy link
Author

Choose a reason for hiding this comment

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

Hmm that's a good point. I'll pin the black version. I'm not worried about autoflake and isort since the change I made was an upgrade and I don't have reason to suspect that newer versions are a problem.

Copy link
Author

Choose a reason for hiding this comment

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

(Plus the version constraints in this pyproject.toml match those in the aurelia directory; it's just the aurelia lock file has these later versions)

Choose a reason for hiding this comment

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

sounds good!

@peter-goldstein peter-goldstein merged commit 31aa4aa into prod/1.x May 24, 2025
peter-goldstein added a commit that referenced this pull request May 27, 2025
…230)

In December of 2020, when we diverged from the upstream branch, the client generator could not yet handle $ref in the parameters list. They added that feature in August 2022 but we never pulled it in.

In this PR, I try to pull in the same implementation from the upstream while modifying our repo as little as possible. Only the tests are really hand-written.

* Improve configuration for vscode development

- Set ruler to 120 characters, which is the configured value in black
- Set up vscode to use pytest with the same configuration as task unit
- Turn off mypy in tests. They are so noncompliant that it's useless

* Copy ParameterError from upstream

* Copy ClassName from upstream

* Copy parameter reference handling from upstream

This builds a library of parameter references from the openapi spec and
gives a utility for looking them up.

* Copy build_parameters from upstream

This builds the actual Parameters object from the OpenAPI spec, using
the logic we just added to schemas.

* Copy parameter reference logic from upstream

Intead of ignoring reference-typed parameters, look them up in the
Parameters object.

* Update unit tests

* Update version to 1.0.7

* Remove explicit local python interpreter

* Format launch.json

* eng: Update code to be editable BNCH-125859 (#229)

* Update code to be editable

Fix the prod/1.x branch so it's possible to run the client library
generator from a local clone.

#### Changes to make it work at all:

`pyproject.toml`
- We'll use python 3.9 because 3.7 is dead-dead
- PyYAML <6 isn't installable anymore, thanks to PEP 517
- shellingham 1.5.0 was yanked, so we need a later version
- Include type annotations for PyYAML for good measure

#### Changes to make the generated output match what's already published:

`poetry.lock`
- Downgrade black so it tolerates leading and trailing spaces in triple-
quoted strings
- Upgrade autoflake to match the version in aurelia
- Upgrade isort to match the version in aurelia

`openapi_python_client/templates/pyproject.toml`
- Enforce line length of 110 characters
- Configure isort to use the same settings as in aurelia (don't separate
by "from foo import" vs "import"; don't separate constants-style imports
from other imports)

`openapi_python_client/__init__.py`
- Run autoflake from outside the generated client package. This sidesteps
an issue where [autoflake will crash if it runs against a file called
types.py](https://discuss.python.org/t/my-python-dont-work/80726). We
have a file called types.py.

* Run poetry lock

* Fix unit tests

* Fix flake8

* Remove difficult-to-fix checks and add todo's

* Pin the black version

* Minimal changes to enable debugging
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.

2 participants
0