-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Stablise Python Builds. #4558
Stablise Python Builds. #4558
Conversation
/test connector=connectors/source-intercom-singer
|
/test connector=connectors/destination-postgres
|
/test connector=connectors/source-shopify
|
echo "Building $path" | ||
./gradlew --no-daemon -no-build-cache "$(_to_gradle_path "$path" clean)" | ||
./gradlew --no-daemon -no-build-cache "$(_to_gradle_path "$path" build)" | ||
./gradlew --no-daemon "$(_to_gradle_path "$path" clean)" |
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 doesn't affect anything, since the connectors test/publish builds aren't using any caching and are always started on a new instance. I feel that putting this here is more confusing than not so I'm removing it.
Thanks @davinchia |
What
Attempt to stablise the python test/publish builds. This is a subset of the changes from #4539 that have been pulled out to keep that PR clean.
A week or so ago we started running into the following errors while trying to test or publish python connectors:
This error is happening when Gradle first configures itself, as it is reading all the various build files and trying to build an internal dependency graph. No real building has happened yet.
At that time, our python builds were running on the
ubuntu-latest
github runners. Switching to other types of ubuntu runners did not fix the problem.The best clue I could find was this stackoverflow. The most promising answer here is to increase the heap, which doesn't seem realistic since the Github runners are memory constrained. I was also unable to reproduce this locally or on any ubuntu ec2 machines I spun up.
Since I know gradle configures itself fine on our ec2 runners, I moved the build to run on an ec2 instance instead of the github instances. This solved our Gradle configure issue but presents another problem with the install of
ciso8601
(see this), which is a package some of the singer sources use to parse date time. This is becauseciso8601
doesn't provide wheels and thus requires very specific C libs to be installed on the instance. The github runners have an extensive list of pre-installed software that fulfills these requirements that our minimal ec2 AMIs do not.Things I tried and failed:
At every point in this process, I validated my hypothesis on a parallel ec2 Ubuntu instance (funny enough, this always worked) built on top of our initial minimal AMI. So I inadvertently created an AMI that contains all the build dependencies Airbyte requires. This is the final AMI we use here. I think this is an okay medium-term solution. The main risk I see from using our own AMI is 1) updates 2) making sure the AMI image can always be built. This ticket contains all this context + is a follow up to address this when we have more time.
This exercise has made me realise we have 3 distinct build runtimes:
I've ran a test from each connector 'type' to prove this set up works. Note: the shopify test is failing from actual failures and not compile/build failures.
Added follows up in #4559 .
Pre-merge Checklist
Expand the checklist which is relevant for this PR.
Connector checklist
airbyte_secret
in the connector's spec./gradlew :airbyte-integrations:connectors:<name>:integrationTest
./test connector=connectors/<name>
command as documented here is passing.README.md
docs/SUMMARY.md
if it's a new connectordocs/integrations/<source or destination>/<name>
.docs/integrations/...
. See changelog exampledocs/integrations/README.md
contains a reference to the new connector/publish
command described hereConnector Generator checklist
-scaffold
in their name) have been updated with the latest scaffold by running./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates
then checking in your changes