-
Notifications
You must be signed in to change notification settings - Fork 1
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
Conversation
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.
# TODO(BNCH-125868) Re-add safety check | ||
# && safety check --bare\ | ||
# TODO(BNCH-125869) Re-add mypy check | ||
# && mypy openapi_python_client\ |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good!
…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
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
Changes to make the generated output match what's already published:
poetry.lock
openapi_python_client/templates/pyproject.toml
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.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