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

test new AMI #19694

Merged
merged 4 commits into from
Nov 29, 2022
Merged

test new AMI #19694

merged 4 commits into from
Nov 29, 2022

Conversation

git-phu
Copy link
Contributor

@git-phu git-phu commented Nov 22, 2022

Use new github runner AMI that is fully documented here: https://github.com/airbytehq/airbyte-cloud/pull/3445

closes #16265

@@ -8,8 +8,8 @@ inputs:
github-token:
required: true
ec2-image-id:
# github-self-hosted-runner-ubuntu-20-100g-disk-with-cypress-deps
default: "ami-005924fb76f7477ce"
# https://internal-docs.airbyte.io/Things-To-Know/Build-Runner-Images#ami-05b3422b3649a6911
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should we include a link to internal docs in oss?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's fine

@git-phu git-phu temporarily deployed to more-secrets November 22, 2022 18:34 Inactive
@git-phu git-phu temporarily deployed to more-secrets November 22, 2022 18:34 Inactive
@git-phu git-phu temporarily deployed to more-secrets November 22, 2022 19:16 Inactive
@git-phu git-phu temporarily deployed to more-secrets November 22, 2022 19:16 Inactive
@git-phu git-phu marked this pull request as ready for review November 22, 2022 19:16
@git-phu git-phu requested review from davinchia, a team and evantahler November 22, 2022 19:18
@evantahler
Copy link
Contributor

Can you remove the AMI override here (

ec2-image-id: ami-06cf12549e3d9c522
) and then we can test out a /test connector run as well?

@git-phu
Copy link
Contributor Author

git-phu commented Nov 22, 2022

Can you remove the AMI override here (

ec2-image-id: ami-06cf12549e3d9c522

) and then we can test out a /test connector run as well?

@evantahler done!

@git-phu git-phu temporarily deployed to more-secrets November 22, 2022 19:44 Inactive
@git-phu git-phu temporarily deployed to more-secrets November 22, 2022 19:44 Inactive
@evantahler
Copy link
Contributor

evantahler commented Nov 22, 2022

/test connector=connectors/source-github

🕑 connectors/source-github https://github.com/airbytehq/airbyte/actions/runs/3526657107
✅ connectors/source-github https://github.com/airbytehq/airbyte/actions/runs/3526657107
Python tests coverage:

Name                             Stmts   Miss  Cover
----------------------------------------------------
source_github/utils.py              14      0   100%
source_github/github_schema.py    8807      0   100%
source_github/__init__.py            2      0   100%
source_github/graphql.py           145      1    99%
source_github/streams.py           796     44    94%
source_github/source.py            104     27    74%
----------------------------------------------------
TOTAL                             9868     72    99%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       139      5    96%   87, 93, 235, 239-240
	 source_acceptance_test/conftest.py                     196     92    53%   35, 41-43, 48, 54, 60, 66, 72-74, 93, 98-100, 106-108, 114-115, 120-121, 126, 132, 141-150, 156-161, 176, 200, 231, 237, 243-248, 256-261, 269-282, 287-293, 300-311, 318-334
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              398    111    72%   53, 58, 87-95, 100-107, 111-112, 116-117, 299, 337-354, 363-371, 375-380, 386, 419-424, 462-469, 512-514, 517, 582-590, 602-605, 610, 666-667, 673, 676, 712-722, 735-760
	 source_acceptance_test/tests/test_incremental.py       158     14    91%   52-59, 64-77, 240
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       112     50    55%   23-26, 32, 36, 39-68, 71-73, 76-78, 81-83, 86-88, 91-93, 96-114, 148-150
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1563    349    78%

Build Passed

Test summary info:

All Passed

@evantahler
Copy link
Contributor

Dang. Looks like the actions run off the yaml defined in master. In this PR we removed the override

https://github.com/airbytehq/airbyte/pull/19694/files#diff-2476bc10fa0b4bbb9f8634037b4828d5501795b3549ad2ebd82c071f4af13947L64-L65

But, the action still ran with the old AMI:

ec2-image-id: ami-06cf12549e3d9c522

@evantahler
Copy link
Contributor

This PR probably closes #16265

@git-phu git-phu temporarily deployed to more-secrets November 23, 2022 01:30 Inactive
@git-phu git-phu temporarily deployed to more-secrets November 23, 2022 01:30 Inactive
Copy link
Contributor

@evantahler evantahler left a comment

Choose a reason for hiding this comment

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

YOLO since we can't test it I guess 🤷

@git-phu
Copy link
Contributor Author

git-phu commented Nov 29, 2022

gonna merge this now and watch for regressions

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.

The action runner EC2 image used for connector publication may not have enough resource
2 participants