FF: restructure and edit#3312
Conversation
🛡️ Jit Security Scan Results
✅ No security findings were detected in this PR
Security scan by Jit
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b5c5666. Configure here.
| --- | ||
|
|
||
| Redis Feature Form supports multiple providers, secrets provider management, and workspaces. | ||
|
|
There was a problem hiding this comment.
Emptied page leaves stale title and broken link
Medium Severity
All body content was moved out of streaming.md to register-providers.md and manage-workspace.md, but the frontmatter still says title: Providers and workspaces and description: Build stream-backed features with Kafka, streaming transformations, and Redis serving.. This renders as an empty page with a misleading title in site navigation. Additionally, overview.md links to this file as "Connect providers" — that link now lands on a blank page. The frontmatter needs updating (or the file needs a redirect/removal).
Reviewed by Cursor Bugbot for commit b5c5666. Configure here.
| description: | ||
| linkTitle: | ||
| weight: 1 | ||
| --- |
There was a problem hiding this comment.
Page with content has empty frontmatter fields
Medium Severity
manage-workspace.md contains real body content (workspace management commands moved from streaming.md) but its title, description, and linkTitle frontmatter fields are all blank. Unlike the other new placeholder files that are intentionally empty stubs, this page has substantive content. The missing frontmatter means it will render with no page title in navigation and no heading, and register-providers.md already links to it.
Reviewed by Cursor Bugbot for commit b5c5666. Configure here.
|
|
||
| Before you register providers, make sure you have: | ||
|
|
||
| - A workspace. See [Manage workspaces](./manage-workspace.md) for the workspace lifecycle commands. |
There was a problem hiding this comment.
Raw markdown links instead of Hugo relref shortcodes
Medium Severity
The two cross-page links in register-providers.md use raw Markdown relative links (./manage-workspace.md, ./define-and-deploy-features.md) instead of Hugo relref shortcodes. Every other cross-page link in this project uses {{< relref "..." >}}, and there is no custom render-link.html hook in the layouts to rewrite .md paths to proper URLs. These links will be broken in the rendered site because Hugo will pass the .md href through as-is, which won't match the generated page URLs.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b5c5666. Configure here.
dwdougherty
left a comment
dwdougherty
left a comment
There was a problem hiding this comment.
Apart from the issues already identified by Bugbot, LGTM.
|
|
||
| ### Workspace state to remember | ||
|
|
||
| - workspaces have unique names and descriptions |
There was a problem hiding this comment.
Nit: A reading of this phrase could be "both names and descriptions are unique across namespaces" so I might suggest "workspaces have unique names and optional descriptions".
| weight: 30 | ||
| --- | ||
|
|
||
| Register the providers and secret backends a Redis Feature Form workspace needs before you author features or transformations. Providers connect the workspace to external systems for storage, compute, serving, or catalog-backed access, and definitions files reference them by name. |
There was a problem hiding this comment.
Not: Is it necessary to qualify "workspace" with "Redis Feature Form"?
| - Any environment variables your provider commands reference set **in the Feature Form server's environment**, not in your shell. | ||
| - For example, `--pg-password-secret env:PG_PASSWORD` makes the server resolve `PG_PASSWORD` from its own process environment at runtime. For Helm-based deployments, set these through chart values; for binary deployments, export them where the server starts. |
There was a problem hiding this comment.
It would be helpful to use the domain name "secret provider" and mention that a default env secret provider is created along with each workspace. The env secret provider is an environment variable type secret provider; however, users may required another type (e.g. Hashicorp Vault, Kubernetes secrets, or AWS Secrets Manager).
| **Best practice:** keep the default health check on. Registration surfaces connectivity and secret-resolution problems at the point you can fix them, rather than as silent failures during materialization or serving. Reserve `--skip-health-check` for cases where you've already validated the provider through another channel. | ||
| {{< /note >}} | ||
|
|
||
| ## Register Postgres for offline storage |
There was a problem hiding this comment.
"... as offline store" might align more closely with Feature Form's domain vocabulary
|
|
||
| ## Register Postgres for offline storage | ||
|
|
||
| Use Postgres when the workspace needs an offline store and Postgres-backed SQL execution in the same path. As an `offline-store`, Postgres holds the historical feature values that training sets read from. As a `compute` provider, it runs the SQL transformations that produce those values. |
There was a problem hiding this comment.
I'm unclear by what _"... in the same path." means here. Users would register PostgreSQL as an offline provider if they are using it as an OLAP/analytical database, in which case it would likely hold a mixture of raw datasets and processed datasets (i.e. tables).
This is true for all offline providers: regardless of the specific technology (e.g. PostgreSQL, Snowflake, etc.) they represent an org's analytics data, so to begin the work of feature engineering on that data, the first step is registering them with Feature Form.
This sentence helps convey this idea to a degree, but I think the language could be sharpened to better convey the reality:
"Postgres holds the historical feature values that training sets read from. As a
computeprovider, it runs the SQL transformations that produce those values."
For example:
"As an offline store, PostgreSQL holds dataset candidates for feature engineering and will also hold Feature Form-managed datasets such as transformed datasets, training sets and feature views. As a compute provider, PostgreSQL will accept workloads orchestrated by Feature Form, such as SQL transformations on primary datasets."
|
|
||
| Use Postgres when the workspace needs an offline store and Postgres-backed SQL execution in the same path. As an `offline-store`, Postgres holds the historical feature values that training sets read from. As a `compute` provider, it runs the SQL transformations that produce those values. | ||
|
|
||
| The `<release-name>` placeholder in `--pg-host` and in the Redis `--redis-host` stands for your Helm release name. With release name `my-ff`, the bundled Postgres service is `my-ff-featureform-provider-postgres`. If you connect to an external Postgres or Redis instance instead of the bundled chart addons, use that hostname directly. |
There was a problem hiding this comment.
I wonder if it would be better to lead with the example of PostgreSQL not deployed in the same cluster as Feature Form as this will most likely only be true in lab/dev environments. Instead, perhaps our examples should reflect production use case (e.g. RDS/Aurora and Redis Cloud).
|
|
||
| ## Register Redis as the online store | ||
|
|
||
| Use Redis when the workspace needs an online store for low-latency feature serving. As an `online-store`, Redis holds the latest materialized feature values and serves them to applications at inference time. |
There was a problem hiding this comment.
Again, I'm not sure of the language/phrasing "... when the workspace needs ..." as the reality is more like "... when Redis is your inference database ...".
One of Feature Form's core value propositions is it allows you to bring your own data infra, so the reality is less "this workspace needs Redis" than it is "Redis is my org's low-latency inference database"
|
|
||
| Use `--force` on `update` when changing values that may break running workloads, such as host, port, or broker addresses. | ||
|
|
||
| ## Configure secret providers |
There was a problem hiding this comment.
I'm seeing this now after making the comment above. I think this should move to the top of the document because users cannot register providers without first configuring secrets.
3 participants
Note
Low Risk
Low-risk documentation-only restructure, but several new pages contain empty front matter (e.g., missing
title/linkTitle) which could affect site navigation/build output.Overview
Moves the provider/secret-provider/workspace CLI documentation out of
streaming.mdinto new dedicated pages: a fullregister-providers.mdguide and a smallermanage-workspace.mdreference.Adds several new FeatureForm docs stubs (
concepts.md,configure-auth.md,define-and-deploy-features.md,query-data.md,reference.md,serve-features.md,update-features.md) that currently only include front matter placeholders, and leavesstreaming.mdeffectively empty aside from its header.Reviewed by Cursor Bugbot for commit b5c5666. Bugbot is set up for automated code reviews on this repo. Configure here.