[DRAFT - In Process] List Variable #44224
Conversation
| const [filterType, setFilterType] = useState<string | undefined>(undefined); | ||
| const [filterOperator, setFilterOperator] = useState<string | undefined>( | ||
| undefined, | ||
| ); | ||
| const [filterText, setFilterText] = useState<string>(""); |
There was a problem hiding this comment.
Let's use searchParams for table state management.
| } = useVariableServiceGetVariables({ | ||
| limit: pagination.pageSize, | ||
| offset: pagination.pageIndex * pagination.pageSize, | ||
| orderBy, |
There was a problem hiding this comment.
All the filters should be passed to the API in this request. The UI shouldn't filter on its own.
| border="1px solid" | ||
| borderColor="gray.800" | ||
| borderRadius="md" | ||
| color="gray.400" |
There was a problem hiding this comment.
| borderColor="gray.800" | ||
| borderRadius="md" | ||
| color="gray.400" | ||
| display="flex" |
There was a problem hiding this comment.
We might just want to use a <Flex> component instead of box if we have to customize it this much.
Also worth looking at HStack. if it works even better
|
|
||
| const { | ||
| data, | ||
| error: VariableError, |
There was a problem hiding this comment.
| error: VariableError, | |
| error, |
Just error is fine. At least we shouldn't capitalize the first letter of a regular variable
There was a problem hiding this comment.
yaa, sorry for it, I thought to change it, somehow missed.
| </Box> | ||
|
|
||
| <HStack> | ||
| <Button colorPalette="blue"> |
There was a problem hiding this comment.
We can keep these buttons but let's set these as disabled and leave a TODO comment to connect them up later.
| import { ErrorAlert } from "src/components/ErrorAlert"; | ||
| import { CloseButton, Select } from "src/components/ui"; | ||
|
|
||
| const variablesColumn = (): Array<ColumnDef<VariableResponse>> => [ |
There was a problem hiding this comment.
This can just be an array instead of a function that returns an array.
| const variablesColumn = (): Array<ColumnDef<VariableResponse>> => [ | |
| const columns: Array<ColumnDef<VariableResponse>> = [ |
| }, | ||
| { | ||
| accessorKey: "value", | ||
| enableSorting: true, |
There was a problem hiding this comment.
| enableSorting: true, |
true is the default.
| useEffect(() => { | ||
| if (data?.variables) { | ||
| let filtered = data.variables; | ||
|
|
||
| if (filterText.trim()) { | ||
| filtered = filtered.filter((item) => { | ||
| const valueToFilter = | ||
| filterType === "key" ? item.key : (item.value ?? ""); | ||
|
|
||
| switch (filterOperator) { | ||
| case "contains": | ||
| return valueToFilter.includes(filterText); | ||
| case "ends_with": | ||
| return valueToFilter.endsWith(filterText); | ||
| case "equals": | ||
| return valueToFilter === filterText; | ||
| case "not_ends_with": | ||
| return !valueToFilter.endsWith(filterText); | ||
| case "not_starts_with": | ||
| return !valueToFilter.startsWith(filterText); | ||
| case "starts_with": | ||
| return valueToFilter.startsWith(filterText); | ||
| default: | ||
| return true; | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| setFilteredData(filtered); | ||
| } else { | ||
| setFilteredData([]); | ||
| } |
There was a problem hiding this comment.
We should really update the API to handle this.
| <DataTable | ||
| columns={variablesColumn()} | ||
| data={filteredData} | ||
| displayMode="table" |
There was a problem hiding this comment.
table is the default
| displayMode="table" |
| isLoading={isLoading} | ||
| modelName="Variable" | ||
| onStateChange={setTableURLState} | ||
| skeletonCount={undefined} |
There was a problem hiding this comment.
| skeletonCount={undefined} |
We don't need to pass this if we dont want to
| </HStack> | ||
| </HStack> | ||
|
|
||
| <ErrorAlert error={VariableError} /> |
There was a problem hiding this comment.
DataTable already has a prop to show errors. Let's move this there
errorMessage={<ErrorAlert error={error} />}
airflow/ui/src/router.tsx
Outdated
| { | ||
| children: [ | ||
| { element: <Overview />, path: "" }, | ||
| { element: <div>Runs</div>, path: "runs" }, | ||
| { element: <div>Tasks</div>, path: "tasks" }, | ||
| { element: <Events />, path: "events" }, | ||
| { element: <Variables />, path: "variables" }, |
There was a problem hiding this comment.
| { element: <Variables />, path: "variables" }, |
We don't want to show variables at the Dag level
shubhamraj-git
force-pushed
the
issue-43709
branch
from
af97ef9
to
a83729e
Compare
relates: #43709
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.