kaxil commented on code in PR #70498:
URL: https://github.com/apache/airflow/pull/70498#discussion_r3681909905
##########
registry/src/_data/exploreCategoryProviders.js:
##########
@@ -19,17 +19,15 @@
const providersData = require("./providers.json");
const exploreCategories = require("./exploreCategories");
+const { providerMatchesKeyword } = require("./providerKeywordMatch");
module.exports = function () {
const map = {};
for (const category of exploreCategories) {
const matched = [];
for (const provider of providersData.providers) {
for (const keyword of category.keywords) {
- if (
- provider.id.includes(keyword) ||
- keyword.includes(provider.id)
- ) {
+ if (providerMatchesKeyword(provider, keyword)) {
matched.push(provider);
Review Comment:
`matched` comes out in `providers.json` order and `explore.njk:32` renders
`| slice(0, 6)` with no sort, so widening membership changes which six badges
show. Running both matchers over all 105 in-tree providers and applying the
same slice: Messaging & Notifications drops `slack` and `smtp` from the row
while the card's own copy still reads "Slack, email, SMS, and alerting
services"; Databases drops `mongo`/`jdbc`/`influxdb`, Data Warehouses
`teradata`/`trino`, Data Processing `apache-livy`/`apache-kylin`, Workflow
`http`/`sftp`, AI & Machine Learning `qdrant`.
This isn't the capping you rejected, nothing loses membership, the badge row
just shows a different six. Sorting `matched` here by `pypi_downloads.monthly`
before it reaches the template would keep the well-known names in the row;
`explore.njk:68` and `:108` already rank their rows before slicing instead of
taking the first N.
##########
registry/src/_data/providerKeywordMatch.js:
##########
@@ -0,0 +1,58 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Shared by providerCategoryMap.js and exploreCategoryProviders.js so the
+// two directions of the same provider <-> explore-category matching (used
+// respectively by the per-provider page's category chips and the Explore
Review Comment:
The prose describing this rule didn't move with it. `README.md:259` still
calls `keywords` a "List of substrings matched against `provider.id`" and
262-264 repeats "matches (substring) the provider's ID". That's what the next
person reads before adding a keyword, and it now understates the match surface.
The PR body also still promises the
`connection_types[].external_integrations` scan that came out last round, and
this comment credits "the per-provider page's category chips" when
`providerCategoryMap`'s only consumer is `providers.njk:65` feeding the
/providers/ dropdown (`provider-detail.njk` has no `categor` match at all).
##########
registry/src/providers.njk:
##########
@@ -68,7 +68,8 @@ mainClass: providers-page
data-name="{{ provider.name | lower }}"
data-downloads="{{ provider.pypi_downloads.monthly }}"
data-updated="{{ provider.last_updated or '' }}"
- data-categories="{{ cats | join(',') }}">
+ data-categories="{{ cats | join(',') }}"
+ data-integrations="{{ (provider.categories or []) |
selectattr('name') | join(',', 'name') | lower }}">
Review Comment:
The client mirror of `collectSearchableValues` is missing the id half: the
build-time matcher searches `provider.id` plus the integration names, but the
box only sees `data-name` (the display name) and this attribute. Sweeping all
105 in-tree providers as queries, not one is findable by the
`apache-airflow-providers-<id>` string `provider-card.njk:20` prints under the
title, and `clickhousedb`, `cncf-kubernetes`, `common-dataquality`, `edge3`,
`microsoft-mssql`, `microsoft-psrp` and `microsoft-winrm` don't resolve from
their bare id either.
Same card-text-vs-search-text mismatch as
https://github.com/apache/airflow/pull/70498#discussion_r3677876974, so happy
for it to ride along in that follow-up rather than here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
