Visitar URL original
feat: Enhance DataSource and FeatureView modals with error handling a… · feast-dev/feast@96d7169 · GitHub
Skip to content

Commit 96d7169

Browse files
rohit-bharmalntkathole
rohit-bharmal
authored and
ntkathole
committed
feat: Enhance DataSource and FeatureView modals with error handling and submission states
Signed-off-by: Rohit Bharmal <rohitbharmal123@gmail.com>
1 parent 01241e4 commit 96d7169

7 files changed

Lines changed: 280 additions & 71 deletions

File tree

‎ui/src/components/DataSourceFormModal.tsx‎

Lines changed: 155 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
EuiSpacer,
77
EuiHorizontalRule,
88
EuiText,
9+
EuiCallOut,
910
} from "@elastic/eui";
1011
import { feast } from "../protos";
1112
import FormModal from "./forms/FormModal";
@@ -75,6 +76,8 @@ interface DataSourceFormModalProps {
7576
onSubmit: (data: DataSourceFormData) => void;
7677
initialData?: DataSourceFormData;
7778
isEdit?: boolean;
79+
isSubmitting?: boolean;
80+
submitError?: string | null;
7881
}
7982

8083
const EMPTY_FORM: DataSourceFormData = {
@@ -100,11 +103,21 @@ const EMPTY_FORM: DataSourceFormData = {
100103
sparkPath: "",
101104
};
102105

106+
const BATCH_SOURCE_TYPES = new Set([
107+
String(feast.core.DataSource.SourceType.BATCH_FILE),
108+
String(feast.core.DataSource.SourceType.BATCH_BIGQUERY),
109+
String(feast.core.DataSource.SourceType.BATCH_SNOWFLAKE),
110+
String(feast.core.DataSource.SourceType.BATCH_REDSHIFT),
111+
String(feast.core.DataSource.SourceType.BATCH_SPARK),
112+
]);
113+
103114
const DataSourceFormModal: React.FC<DataSourceFormModalProps> = ({
104115
onClose,
105116
onSubmit,
106117
initialData,
107118
isEdit = false,
119+
isSubmitting = false,
120+
submitError,
108121
}) => {
109122
const [formData, setFormData] = useState<DataSourceFormData>(
110123
initialData || EMPTY_FORM,
@@ -118,8 +131,11 @@ const DataSourceFormModal: React.FC<DataSourceFormModalProps> = ({
118131
}
119132
}, [initialData]);
120133

134+
const isBatchSource = BATCH_SOURCE_TYPES.has(formData.sourceType);
135+
121136
const validate = (): boolean => {
122137
const newErrors: Record<string, string> = {};
138+
const st = formData.sourceType;
123139

124140
if (!formData.name.trim()) {
125141
newErrors.name = "Data source name is required.";
@@ -128,30 +144,70 @@ const DataSourceFormModal: React.FC<DataSourceFormModalProps> = ({
128144
"Must start with a letter or underscore, and contain only letters, numbers, and underscores.";
129145
}
130146

131-
const st = formData.sourceType;
147+
// Source-type-specific required fields
132148
if (st === String(feast.core.DataSource.SourceType.BATCH_FILE)) {
133149
if (!formData.fileUri.trim()) {
134-
newErrors.fileUri = "File URI is required for file sources.";
150+
newErrors.fileUri = "File URI is required.";
151+
} else if (
152+
!/^(s3|gs|gcs|hdfs|abfs|file):\/\/\S+$/.test(formData.fileUri.trim())
153+
) {
154+
newErrors.fileUri =
155+
"Must be a valid URI (e.g. s3://bucket/path, gs://bucket/path, file:///local/path).";
135156
}
136157
} else if (st === String(feast.core.DataSource.SourceType.BATCH_BIGQUERY)) {
137158
if (!formData.bigqueryTable.trim() && !formData.bigqueryQuery.trim()) {
138-
newErrors.bigqueryTable = "Either table or query is required.";
159+
newErrors.bigqueryTable =
160+
"Either a table reference or a query is required.";
139161
}
140162
} else if (
141163
st === String(feast.core.DataSource.SourceType.BATCH_SNOWFLAKE)
142164
) {
143165
if (!formData.snowflakeTable.trim()) {
144166
newErrors.snowflakeTable = "Table name is required for Snowflake.";
145167
}
168+
if (!formData.snowflakeDatabase.trim()) {
169+
newErrors.snowflakeDatabase = "Database is required for Snowflake.";
170+
}
171+
} else if (st === String(feast.core.DataSource.SourceType.BATCH_REDSHIFT)) {
172+
if (!formData.redshiftTable.trim()) {
173+
newErrors.redshiftTable = "Table name is required for Redshift.";
174+
}
175+
if (!formData.redshiftDatabase.trim()) {
176+
newErrors.redshiftDatabase = "Database is required for Redshift.";
177+
}
178+
} else if (st === String(feast.core.DataSource.SourceType.BATCH_SPARK)) {
179+
if (!formData.sparkTable.trim() && !formData.sparkPath.trim()) {
180+
newErrors.sparkTable =
181+
"Either a table reference or a path is required for Spark.";
182+
}
146183
} else if (st === String(feast.core.DataSource.SourceType.STREAM_KAFKA)) {
147184
if (!formData.kafkaBootstrapServers.trim()) {
148185
newErrors.kafkaBootstrapServers = "Bootstrap servers are required.";
186+
} else if (
187+
!/^[\w.-]+:\d+(,[\w.-]+:\d+)*$/.test(
188+
formData.kafkaBootstrapServers.trim(),
189+
)
190+
) {
191+
newErrors.kafkaBootstrapServers =
192+
"Must be in host:port format (e.g. localhost:9092).";
149193
}
150194
if (!formData.kafkaTopic.trim()) {
151195
newErrors.kafkaTopic = "Topic is required.";
152196
}
153197
}
154198

199+
// Timestamp field required for batch sources (needed for point-in-time correctness)
200+
if (isBatchSource && !formData.timestampField.trim()) {
201+
newErrors.timestampField =
202+
"Timestamp field is required for batch sources. It is used for point-in-time correct feature retrieval.";
203+
} else if (
204+
formData.timestampField.trim() &&
205+
!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(formData.timestampField.trim())
206+
) {
207+
newErrors.timestampField =
208+
"Must be a valid column name (letters, numbers, underscores).";
209+
}
210+
155211
const tagKeys = formData.tags.map((t) => t.key).filter((k) => k.trim());
156212
if (new Set(tagKeys).size !== tagKeys.length) {
157213
newErrors.tags = "Tag keys must be unique.";
@@ -195,7 +251,7 @@ const DataSourceFormModal: React.FC<DataSourceFormModalProps> = ({
195251
label="File URI"
196252
isInvalid={!!errors.fileUri}
197253
error={errors.fileUri}
198-
helpText="s3://path/to/file, gs://path/to/file, or file:///local/path"
254+
helpText="Path to parquet or CSV file(s). Supports s3://, gs://, and file:// schemes."
199255
>
200256
<EuiFieldText
201257
value={formData.fileUri}
@@ -225,12 +281,12 @@ const DataSourceFormModal: React.FC<DataSourceFormModalProps> = ({
225281
</EuiFormRow>
226282
<EuiFormRow
227283
label="Query"
228-
helpText="Alternative to table: a SQL query"
284+
helpText="Optional SQL query — use instead of a fixed table reference."
229285
>
230286
<EuiFieldText
231287
value={formData.bigqueryQuery}
232288
onChange={(e) => updateField("bigqueryQuery", e.target.value)}
233-
placeholder="SELECT * FROM ..."
289+
placeholder="SELECT * FROM `project.dataset.table`"
234290
/>
235291
</EuiFormRow>
236292
</>
@@ -241,21 +297,14 @@ const DataSourceFormModal: React.FC<DataSourceFormModalProps> = ({
241297
return (
242298
<>
243299
<EuiFormRow
244-
label="Table"
245-
isInvalid={!!errors.snowflakeTable}
246-
error={errors.snowflakeTable}
300+
label="Database"
301+
isInvalid={!!errors.snowflakeDatabase}
302+
error={errors.snowflakeDatabase}
247303
>
248-
<EuiFieldText
249-
value={formData.snowflakeTable}
250-
onChange={(e) => updateField("snowflakeTable", e.target.value)}
251-
isInvalid={!!errors.snowflakeTable}
252-
placeholder="MY_TABLE"
253-
/>
254-
</EuiFormRow>
255-
<EuiFormRow label="Database">
256304
<EuiFieldText
257305
value={formData.snowflakeDatabase}
258306
onChange={(e) => updateField("snowflakeDatabase", e.target.value)}
307+
isInvalid={!!errors.snowflakeDatabase}
259308
placeholder="MY_DATABASE"
260309
/>
261310
</EuiFormRow>
@@ -266,24 +315,34 @@ const DataSourceFormModal: React.FC<DataSourceFormModalProps> = ({
266315
placeholder="PUBLIC"
267316
/>
268317
</EuiFormRow>
318+
<EuiFormRow
319+
label="Table"
320+
isInvalid={!!errors.snowflakeTable}
321+
error={errors.snowflakeTable}
322+
>
323+
<EuiFieldText
324+
value={formData.snowflakeTable}
325+
onChange={(e) => updateField("snowflakeTable", e.target.value)}
326+
isInvalid={!!errors.snowflakeTable}
327+
placeholder="MY_TABLE"
328+
/>
329+
</EuiFormRow>
269330
</>
270331
);
271332
}
272333

273334
if (st === String(feast.core.DataSource.SourceType.BATCH_REDSHIFT)) {
274335
return (
275336
<>
276-
<EuiFormRow label="Table">
277-
<EuiFieldText
278-
value={formData.redshiftTable}
279-
onChange={(e) => updateField("redshiftTable", e.target.value)}
280-
placeholder="my_table"
281-
/>
282-
</EuiFormRow>
283-
<EuiFormRow label="Database">
337+
<EuiFormRow
338+
label="Database"
339+
isInvalid={!!errors.redshiftDatabase}
340+
error={errors.redshiftDatabase}
341+
>
284342
<EuiFieldText
285343
value={formData.redshiftDatabase}
286344
onChange={(e) => updateField("redshiftDatabase", e.target.value)}
345+
isInvalid={!!errors.redshiftDatabase}
287346
placeholder="my_database"
288347
/>
289348
</EuiFormRow>
@@ -294,6 +353,18 @@ const DataSourceFormModal: React.FC<DataSourceFormModalProps> = ({
294353
placeholder="public"
295354
/>
296355
</EuiFormRow>
356+
<EuiFormRow
357+
label="Table"
358+
isInvalid={!!errors.redshiftTable}
359+
error={errors.redshiftTable}
360+
>
361+
<EuiFieldText
362+
value={formData.redshiftTable}
363+
onChange={(e) => updateField("redshiftTable", e.target.value)}
364+
isInvalid={!!errors.redshiftTable}
365+
placeholder="my_table"
366+
/>
367+
</EuiFormRow>
297368
</>
298369
);
299370
}
@@ -305,14 +376,15 @@ const DataSourceFormModal: React.FC<DataSourceFormModalProps> = ({
305376
label="Bootstrap Servers"
306377
isInvalid={!!errors.kafkaBootstrapServers}
307378
error={errors.kafkaBootstrapServers}
379+
helpText="Comma-separated list of broker host:port pairs."
308380
>
309381
<EuiFieldText
310382
value={formData.kafkaBootstrapServers}
311383
onChange={(e) =>
312384
updateField("kafkaBootstrapServers", e.target.value)
313385
}
314386
isInvalid={!!errors.kafkaBootstrapServers}
315-
placeholder="localhost:9092"
387+
placeholder="broker1:9092,broker2:9092"
316388
/>
317389
</EuiFormRow>
318390
<EuiFormRow
@@ -334,14 +406,23 @@ const DataSourceFormModal: React.FC<DataSourceFormModalProps> = ({
334406
if (st === String(feast.core.DataSource.SourceType.BATCH_SPARK)) {
335407
return (
336408
<>
337-
<EuiFormRow label="Table">
409+
<EuiFormRow
410+
label="Table"
411+
isInvalid={!!errors.sparkTable}
412+
error={errors.sparkTable}
413+
helpText="Spark catalog table reference (catalog.database.table). Provide either table or path."
414+
>
338415
<EuiFieldText
339416
value={formData.sparkTable}
340417
onChange={(e) => updateField("sparkTable", e.target.value)}
418+
isInvalid={!!errors.sparkTable}
341419
placeholder="catalog.database.table"
342420
/>
343421
</EuiFormRow>
344-
<EuiFormRow label="Path" helpText="Alternative to table">
422+
<EuiFormRow
423+
label="Path"
424+
helpText="Alternative to table: path to data files (s3://, gs://, hdfs://)."
425+
>
345426
<EuiFieldText
346427
value={formData.sparkPath}
347428
onChange={(e) => updateField("sparkPath", e.target.value)}
@@ -366,22 +447,33 @@ const DataSourceFormModal: React.FC<DataSourceFormModalProps> = ({
366447
return null;
367448
};
368449

369-
const isBatchSource =
370-
formData.sourceType !==
371-
String(feast.core.DataSource.SourceType.STREAM_KAFKA) &&
372-
formData.sourceType !==
373-
String(feast.core.DataSource.SourceType.REQUEST_SOURCE) &&
374-
formData.sourceType !==
375-
String(feast.core.DataSource.SourceType.PUSH_SOURCE);
376-
377450
return (
378451
<FormModal
379452
title={isEdit ? "Edit Data Source" : "Create Data Source"}
380453
submitLabel={isEdit ? "Update Data Source" : "Create Data Source"}
381454
onClose={onClose}
382455
onSubmit={handleSubmit}
383456
width={650}
457+
isSubmitting={isSubmitting}
384458
>
459+
{submitError && (
460+
<>
461+
<EuiCallOut
462+
title={
463+
isEdit
464+
? "Unable to update data source"
465+
: "Unable to create data source"
466+
}
467+
color="danger"
468+
iconType="alert"
469+
size="s"
470+
>
471+
<p>{submitError}</p>
472+
</EuiCallOut>
473+
<EuiSpacer size="m" />
474+
</>
475+
)}
476+
385477
<NameDescriptionOwnerFields
386478
name={formData.name}
387479
description={formData.description}
@@ -396,11 +488,31 @@ const DataSourceFormModal: React.FC<DataSourceFormModalProps> = ({
396488
descriptionPlaceholder="Describe this data source..."
397489
/>
398490

399-
<EuiFormRow label="Source Type" helpText="Type of the data source.">
491+
<EuiFormRow
492+
label="Source Type"
493+
helpText="The type of underlying storage system for this data source."
494+
>
400495
<EuiSelect
401496
options={SOURCE_TYPE_OPTIONS}
402497
value={formData.sourceType}
403-
onChange={(e) => updateField("sourceType", e.target.value)}
498+
onChange={(e) => {
499+
updateField("sourceType", e.target.value);
500+
// Clear source-specific errors when type changes
501+
setErrors((prev) => {
502+
const next = { ...prev };
503+
delete next.fileUri;
504+
delete next.bigqueryTable;
505+
delete next.snowflakeTable;
506+
delete next.snowflakeDatabase;
507+
delete next.redshiftTable;
508+
delete next.redshiftDatabase;
509+
delete next.kafkaBootstrapServers;
510+
delete next.kafkaTopic;
511+
delete next.sparkTable;
512+
delete next.timestampField;
513+
return next;
514+
});
515+
}}
404516
disabled={isEdit}
405517
/>
406518
</EuiFormRow>
@@ -419,17 +531,20 @@ const DataSourceFormModal: React.FC<DataSourceFormModalProps> = ({
419531
<EuiSpacer size="m" />
420532
<EuiFormRow
421533
label="Timestamp Field"
422-
helpText="Event timestamp column name."
534+
isInvalid={!!errors.timestampField}
535+
error={errors.timestampField}
536+
helpText="Column containing the event timestamp. Required for point-in-time correct feature retrieval."
423537
>
424538
<EuiFieldText
425539
value={formData.timestampField}
426540
onChange={(e) => updateField("timestampField", e.target.value)}
541+
isInvalid={!!errors.timestampField}
427542
placeholder="event_timestamp"
428543
/>
429544
</EuiFormRow>
430545
<EuiFormRow
431546
label="Created Timestamp Column"
432-
helpText="Column tracking when the row was created."
547+
helpText="Optional: column tracking when the row was written (used for deduplication)."
433548
>
434549
<EuiFieldText
435550
value={formData.createdTimestampColumn}

0 commit comments

Comments
 (0)