-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Airbyte-ci: Increase airbyte-ci start time from ~11s to 0.8s #31488
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
7d3470e
to
64a4337
Compare
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.
"build": "pipelines.airbyte_ci.connectors.build_image.commands.build", | ||
"test": "pipelines.airbyte_ci.connectors.test.commands.test", | ||
"list": "pipelines.airbyte_ci.connectors.list.commands.list", | ||
"publish": "pipelines.airbyte_ci.connectors.publish.commands.publish", | ||
"bump_version": "pipelines.airbyte_ci.connectors.bump_version.commands.bump_version", | ||
"migrate_to_base_image": "pipelines.airbyte_ci.connectors.migrate_to_base_image.commands.migrate_to_base_image", | ||
"upgrade_base_image": "pipelines.airbyte_ci.connectors.upgrade_base_image.commands.upgrade_base_image", |
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 makes me sad because its more things the IDE can't refactor for us 😥
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.
I know :(
This was a solution recommended by click itself so I went with it
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.
Understandable!
# lazy_subcommands is a map of the form: | ||
# | ||
# {command-name} -> {module-name}.{command-object-name} | ||
# |
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.
Should we give it a Optional[Dict[str, str]]
type to help reinforce this?
# check the result to make debugging easier | ||
if not isinstance(cmd_object, click.BaseCommand): | ||
raise ValueError(f"Lazy loading of {import_path} failed by returning " "a non-command object") |
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.
❤️
…hq#31488) Co-authored-by: bnchrch <bnchrch@users.noreply.github.com>
…hq#31488) Co-authored-by: bnchrch <bnchrch@users.noreply.github.com>
Problem
Python loads the kitchen sink by default and Click (our ci library) loads the entire kitchen
Solution
Introduce a lazy loading group as recommended by Click to lazily load command groups to save start up time
Before
![image](https://private-user-images.githubusercontent.com/1325608/275687271-a76dbca6-cf73-4c99-9f72-a2b9040871e2.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTk4NjA3OTYsIm5iZiI6MTcxOTg2MDQ5NiwicGF0aCI6Ii8xMzI1NjA4LzI3NTY4NzI3MS1hNzZkYmNhNi1jZjczLTRjOTktOWY3Mi1hMmI5MDQwODcxZTIucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDcwMSUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDA3MDFUMTkwMTM2WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9M2RiOTZlMjRkNTEzZmIyODhiZjRhNDQ2MGE4ZDE1YWM3YWExZWNiYThhMTgzMDExMmJiNjI5MDQ2ZDU2N2EwNyZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.LKYv4GT9tq4ESrsa2s9997JAwmUwi6F-m3nH0DbWIbw)
After
Notes for reviewer
The largest noticable speed improvement is
airbyte-ci --version
The slowest still will be
airbyte-ci --help
as it needs to load all commands just as beforeand `airbyte-ci connector test should be about 3s-6s faster
closes #31253