🐛 Source S3: fix bug when importing parquet files with null datetimes

[dtiesling]

What

Closes #34151

When importing a parquet file from S3 an exception is raised if a datetime column has a null value.

source > Error parsing record. This could be due to a mismatch between the config's file type and the actual file type, or because the file or record is not parseable. stream=orderAllocation file={path-to=s3-file.parquet} line_no=2 n_skipped=0
Stack Trace: Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/airbyte_cdk/sources/file_based/stream/default_file_based_stream.py", line 90, in read_records_from_slice
    for record in parser.parse_records(self.config, file, self.stream_reader, self.logger, schema):
  File "/usr/local/lib/python3.9/site-packages/airbyte_cdk/sources/file_based/file_types/parquet_parser.py", line 74, in parse_records
    **{
  File "/usr/local/lib/python3.9/site-packages/airbyte_cdk/sources/file_based/file_types/parquet_parser.py", line 75, in <dictcomp></dictcomp>
    column: ParquetParser._to_output_value(batch.column(column)[row], parquet_format)
  File "/usr/local/lib/python3.9/site-packages/airbyte_cdk/sources/file_based/file_types/parquet_parser.py", line 96, in _to_output_value
    return parquet_value.as_py().isoformat()
AttributeError: 'NoneType' object has no attribute 'isoformat'

How

Refactors the parquet_parser _to_output_value method to test for None values and return early. This likely solves for the same bug in other column types.

Recommended reading order

1. parquet_parser.py

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user?

No changes. Expected behavior should start occurring.

For connector PRs, use this section to explain which type of semantic versioning bump occurs as a result of the changes. Refer to our Semantic Versioning for Connectors guidelines for more information. Breaking changes to connectors must be documented by an Airbyte engineer (PR author, or reviewer for community PRs) by using the Breaking Change Release Playbook.

Should be a point release since this is a bug fix only.

If there are breaking changes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

No breaking changes.

Pre-merge Actions

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

• Community member? Grant edit access to maintainers (instructions)
• Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
• Connector version is set to 0.0.1
  • Dockerfile has version 0.0.1
• Documentation updated
  • Connector's README.md
  • Connector's bootstrap.md. See description and examples
  • docs/integrations/<source or destination>/<name>.md including changelog with an entry for the initial version. See changelog example
  • docs/integrations/README.md

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

• Create a non-forked branch based on this PR and test the below items on it
• Build is successful
• If new credentials are required for use in CI, add them to GSM. Instructions.

Updating a connector

Community member or Airbyter

• Grant edit access to maintainers (instructions)
• Unit & integration tests added

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

• Create a non-forked branch based on this PR and test the below items on it
• Build is successful
• If new credentials are required for use in CI, add them to GSM. Instructions.

Connector Generator

• Issue acceptance criteria met
• PR name follows PR naming conventions
• If adding a new generator, add it to the list of scaffold modules being tested
• The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:generateScaffolds then checking in your changes
• Documentation which references the generator is updated as needed

Updating the Python CDK

Airbyter

Before merging:

• Pull Request description explains what problem it is solving
• Code change is unit tested
• Build and my-py check pass
• Smoke test the change on at least one affected connector
  • On Github: Run this workflow, passing --use-local-cdk --name=source-<connector> as options
  • Locally: airbyte-ci connectors --use-local-cdk --name=source-<connector> test
• PR is reviewed and approved

After merging:

• Publish the CDK
  • The CDK does not follow proper semantic versioning. Choose minor if this the change has significant user impact or is a breaking change. Choose patch otherwise.
  • Write a thoughtful changelog message so we know what was updated.
• Merge the platform PR that was auto-created for updating the Connector Builder's CDK version
  • This step is optional if the change does not affect the connector builder or declarative connectors.

[vercel]

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	Feb 27, 2024 4:38am

[erohmensing]

@dtiesling thanks for the fix! Didn't realize we were working on this at the same time (I had a different issue as an entrypoint into investigating this). I have a stack of 2 PRs here that I think resolves the issue in the same way. Do you agree? If so, I'd continue there as it also addresses some typing problems and tests the None case for all the scalar types.

[ambirdsall]

It would be best to remove this change from the PR: since the CDK publication workflow automatically updates airbyte-cdk/python/CHANGELOG.md, it's best to rely on that rather than manually updating—that way there are no git conflicts between updates that are developed simultaneously, the changelog and version tags will be properly sorted in order of publication.

[marcosmarxm]

@dtiesling did you have time to take a look in Alex's comments? Also can you solve the conflicts?

[marcosmarxm]

Closed due inactivity. @dtiesling let me know if you want to return the work here.