Remove Definition File References from Java files (LocalDefinitionProvider), shell scripts and docs #25592

bnchrch · 2023-04-26T23:27:01Z

What

Updates our java code to use the remote registry and spec mask instead
Removes definition and spec file references from markdown files
Remove auto version bump.

closes #25540
closes #25587
closes #25098
related to #24030

Notes for reviewer

This is a big bandaid rip! But note its not targeting master!

github-actions · 2023-05-08T22:24:29Z

Affected Connector Report

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to do the following as needed:

Run integration tests
Bump connector or module version
Add changelog
Publish the new version

✅ Sources (1)

Connector	Version	Changelog	Publish
`source-railz`	`0.1.1`	✅	✅

See "Actionable Items" below for how to resolve warnings and errors.

✅ Destinations (0)

Connector	Version	Changelog	Publish

See "Actionable Items" below for how to resolve warnings and errors.

✅ Other Modules (0)

Actionable Items

(click to expand)

Category	Status	Actionable Item
Version	❌ mismatch	The version of the connector is different from its normal variant. Please bump the version of the connector.
	⚠ doc not found	The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like `source-jdbc` is not published or documented). Please double-check to make sure that it is not a bug.
Changelog	⚠ doc not found	The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like `source-jdbc` is not published or documented). Please double-check to make sure that it is not a bug.
	❌ changelog missing	There is no chnagelog for the current version of the connector. If you are the author of the current version, please add a changelog.
Publish	⚠ not in seed	The connector is not in the cloud or oss registry, so its publication status cannot be checked. This can be normal (e.g. some connectors are cloud-specific, and only listed in the cloud seed file). Please double-check to make sure that you have added a metadata.yaml file and the expected registries are enabled.

alafanechere

👍 Only minor comments. Could you please share your plan on how to test everything works before merging the main PR to master?

alafanechere · 2023-05-09T12:28:29Z

airbyte-config-oss/specs-oss/README.md

@@ -1,16 +1,6 @@
-# Generating Seed Connector Specs
+# The Connector Registry and Spec Secret Masks
+The connector registry (list of available connectors) is downloaded at runtime from https://connectors.airbyte.com/files/registries/v0/oss_registry.json


Suggested change

The connector registry (list of available connectors) is downloaded at runtime from https://connectors.airbyte.com/files/registries/v0/oss_registry.json

The connector registry (list of available connectors) is downloaded at start up from https://connectors.airbyte.com/files/registries/v0/oss_registry.json

I might be wrong but I thought we downloaded the registry in the bootstrap phase, a Airbyte startup.

Ah in my head runtime is startup

Is there an explicit difference in airbyte?

alafanechere · 2023-05-09T12:28:32Z

...e-config-oss/init-oss/src/main/java/io/airbyte/configoss/init/RemoteDefinitionsProvider.java

@@ -72,7 +72,7 @@ private Map<UUID, StandardDestinationDefinition> getDestinationDefinitionsMap()
  public StandardSourceDefinition getSourceDefinition(final UUID definitionId) throws ConfigNotFoundException {
    final StandardSourceDefinition definition = getSourceDefinitionsMap().get(definitionId);
    if (definition == null) {
-      throw new ConfigNotFoundException(SeedType.STANDARD_SOURCE_DEFINITION.name(), definitionId.toString());
+      throw new ConfigNotFoundException("remote_registry:source_def", definitionId.toString());


I don't 💯 get the config type naming with :. Is it standard? "remote_registry:source_def"

This was taken directly from the platform version of RemoteDefinitionsProvider. So im assuming so!

alafanechere · 2023-05-09T12:28:33Z

...te-config-oss/init-oss/src/main/java/io/airbyte/configoss/init/LocalDefinitionsProvider.java

@@ -63,7 +64,7 @@ public Map<UUID, StandardDestinationDefinition> getDestinationDefinitionsMap() {
  public StandardSourceDefinition getSourceDefinition(final UUID definitionId) throws ConfigNotFoundException {
    final StandardSourceDefinition definition = getSourceDefinitionsMap().get(definitionId);
    if (definition == null) {
-      throw new ConfigNotFoundException(SeedType.STANDARD_SOURCE_DEFINITION.name(), definitionId.toString());
+      throw new ConfigNotFoundException("local_registry:source_def", definitionId.toString());


Suggested change

throw new ConfigNotFoundException("local_registry:source_def", definitionId.toString());

throw new ConfigNotFoundException(url.toString(), definitionId.toString());

?

Taken from platform: https://github.com/airbytehq/airbyte-platform-internal/blob/master/oss/airbyte-config/init/src/main/java/io/airbyte/config/init/LocalDefinitionsProvider.java#L87

airbyte-config-oss/init-oss/readme.md

alafanechere · 2023-05-09T12:32:17Z

airbyte-config-oss/specs-oss/build.gradle

+    /**
+     * run this once a day. if you want to force this task to run do so with --rerun
+     * e.g. ./gradlew :airbyte-config-oss:specs:downloadConnectorRegistry --info --rerun
+     */
+    inputs.property("todaysDate", new Date().clearTime() )


Do you really want to just run it once a day now? I think it might prevent us to do some quick fixes when we'll roll this out. I might not matter as I don't know if the Gradle cache is persisted in the CI.

I was modelling how we bring down the Connector Registry currently.

However in the registries case (for cloud) during run time we also continually poll for new updates using cron.

For the secret mask we do not do this, because to apply a new secret mask the whole app needs a restart.

Theres an issue on that concern here: MaskedDataInterceptor is set at build time, should be updated at runtime - Research

alafanechere · 2023-05-09T13:02:18Z

...nfig-oss/specs-oss/src/main/java/io/airbyte/configoss/specs/ConnectorSpecMaskDownloader.java

+    FileUtils.copyURLToFile(new URL(REMOTE_SPEC_SECRET_MASK_URL), writePath.toFile(), timeout, timeout);
+  }
+
+}


missing end line.

alafanechere · 2023-05-09T13:05:47Z

...-oss/specs-oss/src/test/java/io/airbyte/configoss/specs/ConnectorSpecMaskDownloaderTest.java

+    final List<String> commonSecretFieldNames = Arrays.asList(
+        "apiKey",
+        "api_key",
+        "password",
+        "service_account_json"
+    );
+
+    final JsonNode propertiesNode = maskContents.get("properties");
+    final List<String> propertiesList = Jsons.object(propertiesNode, new TypeReference<>() {});
+
+    final boolean containsAll = propertiesList.containsAll(commonSecretFieldNames);


nit: I think this kind of test belongs on the spec mask generation side.

sure, this was a more or less contains data and isnt empty! I believe we assert a similar thing in the metadata service but with the intention of validating certain expect fields exist.

Happy to change this to has an array greater than 1 :)

build.gradle

docs/connector-development/tutorials/building-a-python-source.md

Co-authored-by: Augustin <augustin@airbyte.io>

* Remove Definition File References from Python files (#25590) * Remove check_images_exist.sh * Update definitions.py * Update build_report.py * Update tools/bin/ci_integration_workflow_launcher.py * Update tools/bin/ci_check_dependency.py * tools/bin/scan_all_spec_airbyte_secret.py * Remove qa engine references * Revert "Remove check_images_exist.sh" This reverts commit 7675162. * Improve get url function * Add test * remove scan_all_spec_airbyte_secret.py * add additional test * Remove check_images_exist.sh (#25593) * Remove Definition File References from Java files (LocalDefinitionProvider), shell scripts and docs (#25592) * Remove CombinedConnectorCatalogGenerator.java * Update local definition provider * Update local def test * Add spec mask downloader * Make downloader work * Delete generators and add tests * REMOVE THE YAML FILES * Roughly update docs * Update shell scripts * Remove unused * Add connector metadata file doc * Apply suggestions from code review Co-authored-by: Augustin <augustin@airbyte.io> * Additional PR comments * Run format tasks --------- Co-authored-by: Augustin <augustin@airbyte.io> * Remove unused import * bundle registry * Ignore future updates * Update registry * new registry * Readd maskeddatainterceptor * Automated Change * Remove icon validation * Automated Change * Automated Change * Source Amazon Ads: get rid of `fail_on_extra_columns: false` in SAT (#25913) * Source Amazon Ads: small schema fixes * Source Amazon Ads: update changelog * Source Amazon Ads: update unittest * Source Amazon Ads: unittest additional property is boolean * Source Amazon Ads: bump version * auto-bump connector version --------- Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com> * connectors-ci: make spec-cache / metadata bucket and creds not required for pre-release (#26119) * Automated Change --------- Co-authored-by: Augustin <augustin@airbyte.io> Co-authored-by: bnchrch <bnchrch@users.noreply.github.com> Co-authored-by: Roman Yermilov [GL] <86300758+roman-yermilov-gl@users.noreply.github.com> Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>

* Remove Definition File References from Python files (airbytehq#25590) * Remove check_images_exist.sh * Update definitions.py * Update build_report.py * Update tools/bin/ci_integration_workflow_launcher.py * Update tools/bin/ci_check_dependency.py * tools/bin/scan_all_spec_airbyte_secret.py * Remove qa engine references * Revert "Remove check_images_exist.sh" This reverts commit 7675162. * Improve get url function * Add test * remove scan_all_spec_airbyte_secret.py * add additional test * Remove check_images_exist.sh (airbytehq#25593) * Remove Definition File References from Java files (LocalDefinitionProvider), shell scripts and docs (airbytehq#25592) * Remove CombinedConnectorCatalogGenerator.java * Update local definition provider * Update local def test * Add spec mask downloader * Make downloader work * Delete generators and add tests * REMOVE THE YAML FILES * Roughly update docs * Update shell scripts * Remove unused * Add connector metadata file doc * Apply suggestions from code review Co-authored-by: Augustin <augustin@airbyte.io> * Additional PR comments * Run format tasks --------- Co-authored-by: Augustin <augustin@airbyte.io> * Remove unused import * bundle registry * Ignore future updates * Update registry * new registry * Readd maskeddatainterceptor * Automated Change * Remove icon validation * Automated Change * Automated Change * Source Amazon Ads: get rid of `fail_on_extra_columns: false` in SAT (airbytehq#25913) * Source Amazon Ads: small schema fixes * Source Amazon Ads: update changelog * Source Amazon Ads: update unittest * Source Amazon Ads: unittest additional property is boolean * Source Amazon Ads: bump version * auto-bump connector version --------- Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com> * connectors-ci: make spec-cache / metadata bucket and creds not required for pre-release (airbytehq#26119) * Automated Change --------- Co-authored-by: Augustin <augustin@airbyte.io> Co-authored-by: bnchrch <bnchrch@users.noreply.github.com> Co-authored-by: Roman Yermilov [GL] <86300758+roman-yermilov-gl@users.noreply.github.com> Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>

bnchrch changed the base branch from master to metadata-service/feat-remove-definitions April 26, 2023 23:33

bnchrch added 3 commits May 8, 2023 15:19

Remove CombinedConnectorCatalogGenerator.java

ec869ca

Update local definition provider

03fb139

Update local def test

a299697

bnchrch force-pushed the bnchrch/metadata-service/remove-definition-file-references-java branch from c5c58ad to a299697 Compare May 8, 2023 22:20

bnchrch added 6 commits May 8, 2023 16:09

Add spec mask downloader

6ce2740

Make downloader work

3b39fa1

Delete generators and add tests

3237a92

REMOVE THE YAML FILES

97164ad

Roughly update docs

17abdd5

Update shell scripts

f5ff003

octavia-squidington-iii added the area/documentation Improvements or additions to documentation label May 9, 2023

bnchrch changed the title ~~Remove Definition File References from Java files (LocalDefinitionProvider)~~ Remove Definition File References from Java files (LocalDefinitionProvider), shell scripts and docs May 9, 2023

bnchrch added 2 commits May 8, 2023 18:50

Remove unused

b27b8b0

Add connector metadata file doc

fbf3db1

bnchrch marked this pull request as ready for review May 9, 2023 02:59

bnchrch requested review from a team and alafanechere May 9, 2023 02:59

alafanechere approved these changes May 9, 2023

View reviewed changes

bnchrch and others added 3 commits May 9, 2023 13:37

Apply suggestions from code review

56cf360

Co-authored-by: Augustin <augustin@airbyte.io>

Additional PR comments

5d5ab10

Run format tasks

493cf84

bnchrch merged commit c5503e1 into metadata-service/feat-remove-definitions May 9, 2023

bnchrch deleted the bnchrch/metadata-service/remove-definition-file-references-java branch May 9, 2023 23:55

bnchrch mentioned this pull request May 11, 2023

Docs: Metadata add docs ahead of merge #26015

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove Definition File References from Java files (LocalDefinitionProvider), shell scripts and docs #25592

Remove Definition File References from Java files (LocalDefinitionProvider), shell scripts and docs #25592

bnchrch commented Apr 26, 2023 •

edited

Loading

github-actions bot commented May 8, 2023 •

edited

Loading

✅ Sources (1)

✅ Destinations (0)

✅ Other Modules (0)

Actionable Items

alafanechere left a comment

alafanechere May 9, 2023

bnchrch May 9, 2023

bnchrch May 9, 2023

alafanechere May 9, 2023

bnchrch May 9, 2023

alafanechere May 9, 2023

bnchrch May 9, 2023

alafanechere May 9, 2023

bnchrch May 9, 2023

alafanechere May 9, 2023

alafanechere May 9, 2023

bnchrch May 9, 2023

	The connector registry (list of available connectors) is downloaded at runtime from https://connectors.airbyte.com/files/registries/v0/oss_registry.json
	The connector registry (list of available connectors) is downloaded at start up from https://connectors.airbyte.com/files/registries/v0/oss_registry.json

	throw new ConfigNotFoundException("local_registry:source_def", definitionId.toString());
	throw new ConfigNotFoundException(url.toString(), definitionId.toString());

Remove Definition File References from Java files (LocalDefinitionProvider), shell scripts and docs #25592

Remove Definition File References from Java files (LocalDefinitionProvider), shell scripts and docs #25592

Conversation

bnchrch commented Apr 26, 2023 • edited Loading

What

Notes for reviewer

github-actions bot commented May 8, 2023 • edited Loading

Affected Connector Report

✅ Sources (1)

✅ Destinations (0)

✅ Other Modules (0)

Actionable Items

alafanechere left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnchrch commented Apr 26, 2023 •

edited

Loading

github-actions bot commented May 8, 2023 •

edited

Loading