Skip to content

Commit

Permalink
Merge branch 'main' into jean/oss-5963-deadlock-when-deloying-flow-wi…
Browse files Browse the repository at this point in the history
…th-dependencies-that
  • Loading branch information
jeanluciano authored Jan 16, 2025
2 parents c570973 + 5b5043c commit ca4b321
Show file tree
Hide file tree
Showing 15 changed files with 331 additions and 100 deletions.
4 changes: 2 additions & 2 deletions src/prefect/_internal/concurrency/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def __init__(self, *args: Hashable) -> None:
self._task: Optional[asyncio.Task[None]] = None
self._stopped: bool = False
self._started: bool = False
self._key = hash(args)
self._key = hash((self.__class__, *args))
self._lock = threading.Lock()
self._queue_get_thread = WorkerThread(
# TODO: This thread should not need to be a daemon but when it is not, it
Expand Down Expand Up @@ -256,7 +256,7 @@ def instance(cls, *args: Hashable) -> Self:
If an instance already exists with the given arguments, it will be returned.
"""
with cls._instance_lock:
key = hash(args)
key = hash((cls, *args))
if key not in cls._instances:
cls._instances[key] = cls._new_instance(*args)

Expand Down
8 changes: 8 additions & 0 deletions tests/_internal/concurrency/test_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ def test_instance_returns_new_instance_with_unique_key():
assert isinstance(new_instance, MockService)


def test_different_subclasses_have_unique_instances():
instance = MockService.instance()
assert isinstance(instance, MockService)
new_instance = MockBatchedService.instance()
assert new_instance is not instance
assert isinstance(new_instance, MockBatchedService)


def test_instance_returns_same_instance_after_error():
event = threading.Event()

Expand Down
109 changes: 109 additions & 0 deletions ui-v2/src/components/automations/automation-page/automation-page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import { Automation, buildGetAutomationQuery } from "@/api/automations";
import { AutomationEnableToggle } from "@/components/automations/automation-enable-toggle";
import { AutomationsActionsMenu } from "@/components/automations/automations-actions-menu";
import { useDeleteAutomationConfirmationDialog } from "@/components/automations/use-delete-automation-confirmation-dialog";
import { Card } from "@/components/ui/card";
import { DeleteConfirmationDialog } from "@/components/ui/delete-confirmation-dialog";
import { Typography } from "@/components/ui/typography";
import { useSuspenseQuery } from "@tanstack/react-query";
import { NavHeader } from "./nav-header";

type AutomationPageProps = {
id: string;
};

export const AutomationPage = ({ id }: AutomationPageProps) => {
const { data } = useSuspenseQuery(buildGetAutomationQuery(id));
const [dialogState, confirmDelete] = useDeleteAutomationConfirmationDialog();

const handleDelete = () => confirmDelete(data, { shouldNavigate: true });

return (
<>
<div className="flex flex-col gap-4">
<AutomationPageHeader data={data} onDelete={handleDelete} />
<div className="flex flex-col gap-4">
<AutomationDescription data={data} />
<AutomationTrigger data={data} />
<AutomationActions data={data} />
</div>
</div>
<DeleteConfirmationDialog {...dialogState} />
</>
);
};

type AutomationPageHeaderProps = {
data: Automation;
onDelete: () => void;
};

const AutomationPageHeader = ({
data,
onDelete,
}: AutomationPageHeaderProps) => {
return (
<div className="flex items-center justify-between">
<NavHeader name={data.name} />
<div className="flex items-center gap-2">
<AutomationEnableToggle data={data} />
<AutomationsActionsMenu id={data.id} onDelete={onDelete} />
</div>
</div>
);
};

type AutomationDescriptionProps = {
data: Automation;
};

const AutomationDescription = ({ data }: AutomationDescriptionProps) => {
return (
<div className="flex flex-col gap-1">
<Typography className="text-muted-foreground" variant="bodySmall">
Description
</Typography>
<Typography className="text-muted-foreground">
{data.description || "None"}
</Typography>
</div>
);
};

type AutomationTriggerProps = {
data: Automation;
};

const AutomationTrigger = ({ data }: AutomationTriggerProps) => {
const { trigger } = data;
return (
<div className="flex flex-col gap-1">
<Typography>Trigger</Typography>
<Typography variant="bodySmall">
TODO: {JSON.stringify(trigger)}
</Typography>
</div>
);
};

type AutomationActionsProps = {
data: Automation;
};

const AutomationActions = ({ data }: AutomationActionsProps) => {
const { actions } = data;
return (
<div className="flex flex-col gap-1">
<Typography>{`Action${actions.length > 1 ? "s" : ""}`}</Typography>
<ul className="flex flex-col gap-2">
{actions.map((action, i) => (
<li key={i}>
<Card className="p-4 border-r-8">
<div>TODO: {JSON.stringify(action)}</div>
</Card>
</li>
))}
</ul>
</div>
);
};
1 change: 1 addition & 0 deletions ui-v2/src/components/automations/automation-page/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { AutomationPage } from "./automation-page";
32 changes: 32 additions & 0 deletions ui-v2/src/components/automations/automation-page/nav-header.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import {
Breadcrumb,
BreadcrumbItem,
BreadcrumbLink,
BreadcrumbList,
BreadcrumbPage,
BreadcrumbSeparator,
} from "@/components/ui/breadcrumb";

type NavHeaderProps = {
name: string;
};

export const NavHeader = ({ name }: NavHeaderProps) => {
return (
<div className="flex items-center gap-2">
<Breadcrumb>
<BreadcrumbList>
<BreadcrumbItem>
<BreadcrumbLink to="/automations" className="text-xl font-semibold">
Automations
</BreadcrumbLink>
</BreadcrumbItem>
<BreadcrumbSeparator />
<BreadcrumbItem className="text-xl font-semibold">
<BreadcrumbPage>{name}</BreadcrumbPage>
</BreadcrumbItem>
</BreadcrumbList>
</Breadcrumb>
</div>
);
};
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {
Breadcrumb,
BreadcrumbItem,
BreadcrumbLink,
BreadcrumbList,
} from "@/components/ui/breadcrumb";
import { Button } from "@/components/ui/button";
Expand All @@ -22,20 +21,20 @@ const Header = () => (
<div className="flex items-center gap-2">
<Breadcrumb>
<BreadcrumbList>
<BreadcrumbItem>
<BreadcrumbLink
to="/concurrency-limits"
className="text-xl font-semibold"
>
Automations
</BreadcrumbLink>
<BreadcrumbItem className="text-xl font-semibold">
Automations
</BreadcrumbItem>
</BreadcrumbList>
</Breadcrumb>
<Button size="icon" className="h-7 w-7" variant="outline">
<Link to="/automations/create">
<Link to="/automations/create" aria-label="create automation">
<Button
size="icon"
className="h-7 w-7"
variant="outline"
aria-label="create automation"
>
<Icon id="Plus" className="h-4 w-4" />
</Link>
</Button>
</Button>
</Link>
</div>
);
19 changes: 19 additions & 0 deletions ui-v2/src/components/automations/automations-page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { buildListAutomationsQuery } from "@/api/automations";
import { useSuspenseQuery } from "@tanstack/react-query";
import { AutomationsEmptyState } from "./automations-empty-state";
import { AutomationsHeader } from "./automations-header";

export const AutomationsPage = () => {
const { data } = useSuspenseQuery(buildListAutomationsQuery());

return (
<div className="flex flex-col gap-4">
<AutomationsHeader />
{data.length === 0 ? (
<AutomationsEmptyState />
) : (
<div>TODO: AUTOMATIONS_LIST</div>
)}
</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
} from "@/components/ui/form";
import { Skeleton } from "@/components/ui/skeleton";
import { useQuery } from "@tanstack/react-query";
import { useDeferredValue, useMemo, useState } from "react";
import { useFormContext } from "react-hook-form";

const INFER_AUTOMATION = {
Expand All @@ -44,37 +45,35 @@ const getButtonLabel = (
return INFER_AUTOMATION.name;
}
const automation = data?.find((automation) => automation.id === fieldValue);
if (automation?.name) {
if (automation) {
return automation.name;
}
return undefined;
};

/** Because ShadCN only filters by `value` and not by a specific field, we need to write custom logic to filter objects by id */
const filterAutomations = (
value: string | null,
search: string,
data: Array<Automation> | undefined,
) => {
const searchTerm = search.toLowerCase();
const automation = data?.find((automation) => automation.id === value);
if (!automation) {
return 0;
}
const automationName = automation.name.toLowerCase();
if (automationName.includes(searchTerm)) {
return 1;
}
return 0;
};

export const AutomationsSelectStateFields = ({
action,
index,
}: AutomationsSelectStateFieldsProps) => {
const [search, setSearch] = useState("");
const form = useFormContext<AutomationWizardSchema>();
const { data, isSuccess } = useQuery(buildListAutomationsQuery());

// nb: because automations API does not have filtering _like by name, do client-side filtering
const deferredSearch = useDeferredValue(search);
const filteredData = useMemo(() => {
if (!data) {
return [];
}
return data.filter((automation) =>
automation.name.toLowerCase().includes(deferredSearch.toLowerCase()),
);
}, [data, deferredSearch]);

const isInferredOptionFiltered = INFER_AUTOMATION.name
.toLowerCase()
.includes(deferredSearch.toLowerCase());

return (
<FormField
control={form.control}
Expand All @@ -89,30 +88,38 @@ export const AutomationsSelectStateFields = ({
selected={Boolean(buttonLabel)}
aria-label={`Select Automation to ${action}`}
>
{getButtonLabel(data, field.value) ?? "Select automation"}
{buttonLabel ?? "Select automation"}
</ComboboxTrigger>
<ComboboxContent
filter={(value, search) =>
filterAutomations(value, search, data)
}
>
<ComboboxCommandInput placeholder="Search for an automation..." />
<ComboboxContent>
<ComboboxCommandInput
value={search}
onValueChange={setSearch}
placeholder="Search for an automation..."
/>
<ComboboxCommandEmtpy>No automation found</ComboboxCommandEmtpy>
<ComboboxCommandList>
<ComboboxCommandGroup>
<ComboboxCommandItem
selected={field.value === INFER_AUTOMATION.value}
onSelect={field.onChange}
value={INFER_AUTOMATION.value}
>
{INFER_AUTOMATION.name}
</ComboboxCommandItem>
{isInferredOptionFiltered && (
<ComboboxCommandItem
selected={field.value === INFER_AUTOMATION.value}
onSelect={(value) => {
field.onChange(value);
setSearch("");
}}
value={INFER_AUTOMATION.value}
>
{INFER_AUTOMATION.name}
</ComboboxCommandItem>
)}
{isSuccess ? (
data.map((automation) => (
filteredData.map((automation) => (
<ComboboxCommandItem
key={automation.id}
selected={field.value === automation.id}
onSelect={field.onChange}
onSelect={(value) => {
field.onChange(value);
setSearch("");
}}
value={automation.id}
>
{automation.name}
Expand Down
Loading

0 comments on commit ca4b321

Please sign in to comment.