Skip to content

πŸŽ‰ Source S3: Reduce image size and add acceptance test - #31654

Merged
Joe Reuter (flash1293) merged 11 commits into
masterfrom
flash1293/s3-improve-and-test
Oct 25, 2023
Merged

πŸŽ‰ Source S3: Reduce image size and add acceptance test#31654
Joe Reuter (flash1293) merged 11 commits into
masterfrom
flash1293/s3-improve-and-test

Conversation

@flash1293

@flash1293 Joe Reuter (flash1293) commented Oct 20, 2023

Copy link
Copy Markdown
Contributor

Adding the experimental document type parser bloated the image size unnecessarily. This PR reduces the required dependencies.

It also adds an acceptance test to verify the parser works correctly for regular PDFs and OCR-only PDFs (e.g. scans)

⚠️ This PR means that the hi_res parsing strategy can't be applied anymore - it will default to fast and fall back to ocr_only if necessary. ⚠️

@vercel

vercel Bot commented Oct 20, 2023

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated (UTC)
airbyte-docs βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback Oct 25, 2023 10:27am

@github-actions

github-actions Bot commented Oct 20, 2023

Copy link
Copy Markdown
Contributor

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! πŸŽ‰

To merge this PR, ensure the following has been done/considered for each connector added or updated:

  • PR name follows PR naming conventions
  • Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan.
  • Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
  • You've updated the connector's metadata.yaml file any other relevant changes, including a breakingChanges entry for major version bumps. See metadata.yaml docs
  • Secrets in the connector's spec are annotated with airbyte_secret
  • All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
  • Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
  • Migration guide updated in docs/integrations/<source or destination>/<name>-migrations.md with an entry for the new version, if the version is a breaking change. See migration guide example
  • If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

  1. Check for hidden checklists in your PR description

  2. Toggle the github label checklist-action-run on/off to re-run the checklist CI.

@airbyte-oss-build-runner

Copy link
Copy Markdown
Collaborator

source-s3 test report (commit a971df111d) - ❌

⏲️ Total pipeline duration: 15mn10s

Step Result
Build source-s3 docker image for platform(s) linux/x86_64 βœ…
Unit tests βœ…
Acceptance tests ❌
Check our base image is used βœ…
Code format checks βœ…
Validate metadata for source-s3 βœ…
Connector version semver check βœ…
Connector version increment check βœ…
QA checks βœ…

πŸ”— View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-s3 test

@bazarnov Baz (bazarnov) changed the title S3: Reduce image size and add acceptance test πŸŽ‰ Source S3: Reduce image size and add acceptance test Oct 20, 2023
@airbyte-oss-build-runner

Copy link
Copy Markdown
Collaborator

source-s3 test report (commit e76c488b42) - ❌

⏲️ Total pipeline duration: 14mn35s

Step Result
Build source-s3 docker image for platform(s) linux/x86_64 βœ…
Unit tests βœ…
Acceptance tests ❌
Check our base image is used βœ…
Code format checks βœ…
Validate metadata for source-s3 βœ…
Connector version semver check βœ…
Connector version increment check βœ…
QA checks βœ…

πŸ”— View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-s3 test

@airbyte-oss-build-runner

Copy link
Copy Markdown
Collaborator

source-s3 test report (commit eb6941d301) - βœ…

⏲️ Total pipeline duration: 06mn19s

Step Result
Build source-s3 docker image for platform(s) linux/x86_64 βœ…
Unit tests βœ…
Acceptance tests βœ…
Check our base image is used βœ…
Code format checks βœ…
Validate metadata for source-s3 βœ…
Connector version semver check βœ…
Connector version increment check βœ…
QA checks βœ…

πŸ”— View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-s3 test

@flash1293

Copy link
Copy Markdown
Contributor Author

One other thing I need to check here is whether we should pull in pytesseract

@airbyte-oss-build-runner

Copy link
Copy Markdown
Collaborator

source-s3 test report (commit f628f5831e) - ❌

⏲️ Total pipeline duration: 14mn29s

Step Result
Build source-s3 docker image for platform(s) linux/amd64 βœ…
Unit tests βœ…
Acceptance tests βœ…
Check our base image is used βœ…
Code format checks ❌
Validate metadata for source-s3 βœ…
Connector version semver check βœ…
Connector version increment check βœ…
QA checks βœ…

πŸ”— View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-s3 test

@airbyte-oss-build-runner

Copy link
Copy Markdown
Collaborator

source-s3 test report (commit 254aa91d6d) - βœ…

⏲️ Total pipeline duration: 14mn38s

Step Result
Build source-s3 docker image for platform(s) linux/amd64 βœ…
Unit tests βœ…
Acceptance tests βœ…
Check our base image is used βœ…
Code format checks βœ…
Validate metadata for source-s3 βœ…
Connector version semver check βœ…
Connector version increment check βœ…
QA checks βœ…

πŸ”— View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-s3 test

@alafanechere Augustin (alafanechere) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

πŸ‘ Just a suggestion to make the build_customization.py module more readable / documented.

Comment thread airbyte-integrations/connectors/source-s3/build_customization.py Outdated
Comment thread airbyte-integrations/connectors/source-s3/source_s3/v4/unstructured_parser.py Outdated


async def post_connector_install(connector_container: Container) -> Container:
async def setup_nltk(connector_container: Container) -> Container:

@alafanechere Augustin (alafanechere) Oct 23, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These two new functions do not need to be async. (there's no await call inside them).

The post_connector_install has to remain async because we want to allow awaits inside them if necessary (which is not your case)

@airbyte-oss-build-runner

Copy link
Copy Markdown
Collaborator

source-s3 test report (commit efa0727cb8) - βœ…

⏲️ Total pipeline duration: 13mn32s

Step Result
Build source-s3 docker image for platform(s) linux/amd64 βœ…
Unit tests βœ…
Acceptance tests βœ…
Check our base image is used βœ…
Code format checks βœ…
Validate metadata for source-s3 βœ…
Connector version semver check βœ…
Connector version increment check βœ…
QA checks βœ…

πŸ”— View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-s3 test

@flash1293

Copy link
Copy Markdown
Contributor Author

Augustin (@alafanechere) thanks, fixed!

@airbyte-oss-build-runner

Copy link
Copy Markdown
Collaborator

source-s3 test report (commit 1eadd5df96) - βœ…

⏲️ Total pipeline duration: 13mn40s

Step Result
Build source-s3 docker image for platform(s) linux/amd64 βœ…
Unit tests βœ…
Acceptance tests βœ…
Check our base image is used βœ…
Code format checks βœ…
Validate metadata for source-s3 βœ…
Connector version semver check βœ…
Connector version increment check βœ…
QA checks βœ…

πŸ”— View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-s3 test

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@airbyte-oss-build-runner

Copy link
Copy Markdown
Collaborator

source-s3 test report (commit f601bef3d7) - βœ…

⏲️ Total pipeline duration: 16mn06s

Step Result
Build source-s3 docker image for platform(s) linux/amd64 βœ…
Unit tests βœ…
Acceptance tests βœ…
Check our base image is used βœ…
Code format checks βœ…
Validate metadata for source-s3 βœ…
Connector version semver check βœ…
Connector version increment check βœ…
QA checks βœ…

πŸ”— View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-s3 test

@flash1293
Joe Reuter (flash1293) merged commit 68e99ce into master Oct 25, 2023
@flash1293
Joe Reuter (flash1293) deleted the flash1293/s3-improve-and-test branch October 25, 2023 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/s3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants