🚨🚨 [ISSUE #19733] Executing a source with an invalid JSON file should display a clear error

[maxi297]

What

This change address #19733 which would cause the error message from reading config files to be unclear.

How

As #19733 states, We should rename BaseConnector.read_config() to read_json_file() to accurately reflect what it is doing. The existing usages of read_config() should be updated. and The Source.read_state() method should start using this newly renamed read_json_file() to read the state JSON file instead of duplicating all the same file reading logic.

🚨 User Impact 🚨

Previous error message: Expecting value: line 1 column 1 (char 0)
New error message: Could not read json file secrets/config.json: Expecting value: line 1 column 1 (char 0)

The solution proposed in #19733 is a breaking change since the public method read_config is changed to read_json_file.

Pre-merge Checklist

Community member or Airbyter

• Unit & integration tests added and passing
• Code reviews completed
• PR name follows PR naming conventions

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

Other Notes

Running mypy airbyte_cdk (as mentioned in the README.md) before creating changes to the repository gave me the following output Found 134 errors in 60 files (checked 140 source files). Is this expected?

[CLAassistant]

All committers have signed the CLA.

[sherifnada]

Nice work! The main change looks good to me. Request for change: more connectors use this method, so we should make sure to update them as well:

airbyte-integrations/connectors/source-s3/source_s3/source.py:
  60      def read_config(self, config_path: str) -> Mapping[str, Any]:
  61:         config: Mapping[str, Any] = super().read_config(config_path)
  62          if config.get("format", {}).get("delimiter") == r"\t":

airbyte-integrations/connectors/source-s3/unit_tests/conftest.py:
  51      source = SourceS3()
  52:     config = source.read_config(config_file)
  53      return config

airbyte-integrations/connectors/source-s3/unit_tests/test_source.py:
  21      source = SourceS3()
  22:     config = source.read_config(config_file)
  23      assert config["format"]["delimiter"] == "\t"

[brianjlai]

changes and additional tests look good. great work!

Will approve pending the update to the s3 source which appears to be the only source that was using read_config(). I think we also discussed async, but the json file reading in BaseConnector.spec() to reuse what you built here will be in a follow up PR?

[brianjlai]

looks good! just responses to the comments you left

[maxi297]

Since this merge request was from a forked repo, it had to be reopened in the current airbyte repo. Here is the new PR for this change: #20019