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

Error details & Generator improvements #22

Merged
merged 13 commits into from
Nov 22, 2023

Conversation

sherbang
Copy link
Contributor

@sherbang sherbang commented Nov 9, 2023

So I started with some more improvements to providing error details in exceptions, then I got carried away and did a bunch of work on the generator.

Changes include:

  • Add more checks to ruff (and related code changes)
  • Parsing the error that Manage returns when you try to create an object that already exists
  • Moving generator templates out into separate files so code highlighting works
  • Move generator out of pyconnectwise namespace, so it isn't included with the shipped library
  • Document how to run the generator
  • Glued together the different parts of the generator, so it all runs in one go (model gen, schema cleanup, endpoint gen)
  • Changed line-length in our tooling to match what the generator puts out
  • Make sure that we don't generate endpoints that refer to models that don't exist (fail faster)

I think that's everything worth mentioning. Now I've got to figure out/remember what started me on this journey in the first place. 😁

@sherbang
Copy link
Contributor Author

sherbang commented Nov 9, 2023

Nice automation!

This resulted in a memory usage reduction from ~240Mb to ~50Mb and a time reduction from 2.8s to 136ms when running `python -c 'import pyconnectwise'`.
@sherbang
Copy link
Contributor Author

I pushed a few more commits today. The most notable one is lazy-loading endpoints.

Before the lazy-loading endpoints change:
python -c 'import pyconnectwise' would take 2.8 seconds to run and use ~240Mb of memory.

After the change:
python -c 'import pyconnectwise' takes 136 milliseconds to run and use ~50Mb of memory.

@Yoshify
Copy link
Contributor

Yoshify commented Nov 15, 2023

This PR is fantastic - I love the additions to lazy loading the imports. I still have more testing I'd like to do with it (with generating Automate files especially) but other than that I can't see any downsides to this. I expect to merge soon.

Copy link
Contributor

@Yoshify Yoshify left a comment

Choose a reason for hiding this comment

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

LGTM

@Yoshify
Copy link
Contributor

Yoshify commented Nov 22, 2023

Merged - thanks again for your incredible contribution!

@Yoshify Yoshify merged commit cb70636 into HealthITAU:main Nov 22, 2023
2 checks passed
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