[UI] Merge the Conda resource into the Aqueduct Server #1311

kenxu95 · 2023-05-11T23:02:41Z

@likawind as lead reviewer. @agiron123 there are some resource dialog changes.
@vsreekanti to watch the demos.

Describe your changes and why you are making these changes

Instead of treating Conda as a standalone resource, we merge it into the concept of the "Aqueduct Server". See the demos for the full flow.

On the backend, nothing changes except that the returned "Aqueduct Server" entry now has either the server python version or the Conda config embedded inside it.

Related issue number (if any)

ENG-2852
ENG-2871

Loom demo (if any)

https://www.loom.com/share/930b29c23ae648c99d41c48c0b11f23c
http

Checklist before requesting a review

I have created a descriptive PR title. The PR title should complete the sentence "This PR...".
I have performed a self-review of my code.
I have included a small demo of the changes. For the UI, this would be a screenshot or a Loom video.
If this is a new feature, I have added unit tests and integration tests.
I have run the integration tests locally and they are passing.
I have run the linter script locally (See python3 scripts/run_linters.py -h for usage).
All features on the UI continue to work correctly.
Added one of the following CI labels:
- run_integration_test: Runs integration tests
- skip_integration_test: Skips integration tests (Should be used when changes are ONLY documentation/UI)

agiron123 · 2023-05-11T23:19:09Z

src/ui/common/src/components/integrations/dialogs/dialog.tsx

@@ -188,6 +188,13 @@ const IntegrationDialog: React.FC<Props> = ({
    connectionMessage = `Connecting to ${service}`;
  }

+  // For Conda, we provide a default name, and don't allow the user to ever change it.
+  useEffect(() => {
+    if (service == 'Conda') {


likawind

LGTM overall. My understanding is we still return conda as a separate integration, but we hide the conda card in frontend?

likawind · 2023-05-12T17:33:12Z

src/golang/cmd/server/handler/delete_integration.go

@@ -76,6 +76,11 @@ func (h *DeleteIntegrationHandler) Prepare(r *http.Request) (interface{}, int, e
 		}
 	}

+	// Built-in resources cannot be deleted.
+	if integrationObject.Service == shared.Aqueduct || integrationObject.Service == shared.Filesystem || (integrationObject.Name == shared.DemoDbIntegrationName && integrationObject.Service == shared.Sqlite) {


can we do a isBuiltin() helper over Service type?

I can't just do if over the service type because Demo database is a SQLite type, but I can make a helper over name + service.

src/ui/common/src/components/integrations/dialogs/condaDialog.tsx

likawind · 2023-05-15T17:54:32Z

src/golang/lib/execution_environment/python_version.go

+)
+
+// GetServerPythonVersion performs a best-effort fetch of the server's python version. This should only be used when conda is not available.
+func GetServerPythonVersion() string {


How about just func PythonVersion() ? No strong preference

yeah I also don't have a strong preference, but I was told that Get() implies that we're doing more processing before deriving the returned data, which is the case here. I think I slightly prefer the current form though.

likawind

LGTM overall. My understanding is we still return conda as a separate integration, but we hide the conda card in frontend?

agiron123 · 2023-05-15T18:14:49Z

src/ui/common/src/components/integrations/addIntegrations.tsx

+  const resources = useSelector(
+    (state: RootState) => state.integrationsReducer.integrations
+  );
+  if (svc === 'Conda') {


Probably not something that needs to be fixed now, but I wonder if there is a functin we can add to SupportedIntegrations enum that allows for us to get this info from the conda dialog.

kenxu95 · 2023-05-15T21:15:30Z

LGTM overall. My understanding is we still return conda as a separate integration, but we hide the conda card in frontend?
@likawind Correct!

commit e1fd9d7 Merge: 370665e 2fad420 Author: Wei Chen <[email protected]> Date: Wed May 17 14:19:11 2023 -0700 Merge branch 'eng-2699-m1-refactor-wf-detail-pages-to-use-new' into eng-2699-m1-refactor-wf-detail-pages-to-use-new-2 commit 2fad420 Merge: b4485f4 676e81a Author: Wei Chen <[email protected]> Date: Wed May 17 14:06:26 2023 -0700 Merge branch 'eng-2699-m1-refactor-wf-detail-pages-to-use-new' of https://github.com/aqueducthq/aqueduct into eng-2699-m1-refactor-wf-detail-pages-to-use-new commit b4485f4 Author: Wei Chen <[email protected]> Date: Mon Apr 24 15:43:51 2023 -0700 rebase commit d3a2669 Merge: e60ac09 18011cb Author: Wei Chen <[email protected]> Date: Wed May 17 14:04:32 2023 -0700 Merge branch 'eng-2712-m1-migrate-wf-edit-trigger-endpoints-to' into eng-2943-investigate-result-content-route-tests commit 18011cb Merge: ec515cf 8cb356c Author: Wei Chen <[email protected]> Date: Wed May 17 14:02:47 2023 -0700 Merge branch 'eng-2847-m1-implement-wfiddags-endpoint-with' into eng-2712-m1-migrate-wf-edit-trigger-endpoints-to commit 8cb356c Merge: ec94379 b3946a4 Author: Wei Chen <[email protected]> Date: Wed May 17 14:02:23 2023 -0700 Merge branch 'main' of https://github.com/aqueducthq/aqueduct into eng-2847-m1-implement-wfiddags-endpoint-with commit 676e81a Author: Wei Chen <[email protected]> Date: Mon Apr 24 15:43:51 2023 -0700 rebase commit ec94379 Author: Wei Chen <[email protected]> Date: Wed May 17 13:28:38 2023 -0700 comments commit b3946a4 Author: kenxu95 <[email protected]> Date: Wed May 17 10:19:11 2023 -0700 Move any overflow rows on the details header fields into an additional column (#1322) commit e60ac09 Author: Wei Chen <[email protected]> Date: Tue May 16 20:04:49 2023 -0700 fix commit ec515cf Merge: 1d0482a f81c065 Author: Wei Chen <[email protected]> Date: Tue May 16 17:37:04 2023 -0700 Merge branch 'eng-2847-m1-implement-wfiddags-endpoint-with' into eng-2712-m1-migrate-wf-edit-trigger-endpoints-to commit f81c065 Author: Wei Chen <[email protected]> Date: Tue May 16 17:34:45 2023 -0700 lint commit 1d0482a Author: Wei Chen <[email protected]> Date: Tue May 16 17:29:44 2023 -0700 works commit 5ad5130 Author: Andre Giron <[email protected]> Date: Tue May 16 16:34:41 2023 -0700 ENG-2979: Fixes bigquery and gcs dialogs that were crashing after file uploads. (#1326) Co-authored-by: Andre Giron <[email protected]> commit 03aa76e Author: Andre Giron <[email protected]> Date: Tue May 16 16:33:35 2023 -0700 ENG-2979 S3 Dialog Validation Fixes (#1328) Co-authored-by: Andre Giron <[email protected]> commit f63c5f3 Author: kenxu95 <[email protected]> Date: Tue May 16 16:24:14 2023 -0700 Fix Snowflake save missing schema bug (#1331) commit a458ce6 Author: Wei Chen <[email protected]> Date: Tue May 16 15:27:52 2023 -0700 works commit f529c5f Author: Saurav Chhatrapati <[email protected]> Date: Tue May 16 13:36:12 2023 -0400 Fixes cloudpickle serialization for Python 3.7 (#1324) commit c335fd5 Author: Saurav Chhatrapati <[email protected]> Date: Tue May 16 12:15:48 2023 -0400 Adds error checking for whether big query dataset exists (#1319) commit d3521a4 Author: kenxu95 <[email protected]> Date: Mon May 15 20:58:16 2023 -0700 Add ability to parameterize the save operator (#1320) commit 942dec1 Author: kenxu95 <[email protected]> Date: Mon May 15 15:36:21 2023 -0700 [UI] Merge the Conda resource into the Aqueduct Server (#1311) commit ccf4e17 Author: Wei Chen <[email protected]> Date: Mon May 15 14:57:19 2023 -0700 [2/n] Adds gh actions to publish test pypi packages (#1262) commit fb67044 Author: kenxu95 <[email protected]> Date: Mon May 15 12:04:48 2023 -0700 Fix bug where duplicate fields showing in resource details dropdown (#1321) commit cfeeb2f Author: kenxu95 <[email protected]> Date: Mon May 15 11:45:31 2023 -0700 Fix generated docstrings when viewing resource methods in Jupyter (#1315) commit b544252 Author: eunice-chan <[email protected]> Date: Fri May 12 21:26:55 2023 -0500 Eng 2884 test node routes (#1299) Co-authored-by: Eunice Chan <[email protected]> Co-authored-by: Wei Chen <[email protected]> commit a8abb16 Author: Andre Giron <[email protected]> Date: Fri May 12 16:58:59 2023 -0700 Fixes broken storybook build (#1317) Co-authored-by: Andre Giron <[email protected]> commit d6ed5ca Author: kenxu95 <[email protected]> Date: Fri May 12 10:56:42 2023 -0700 Update the workflow "No Run" logo (#1312) commit c9fec70 Author: Chenggang Wu <[email protected]> Date: Fri May 12 09:30:54 2023 -0700 README typo fix (#1313) commit b686014 Author: Andre Giron <[email protected]> Date: Thu May 11 12:49:53 2023 -0700 ENG-2767 react hook form submission (#1232) Co-authored-by: Andre Giron <[email protected]> Co-authored-by: Chenggang Wu <[email protected]> Co-authored-by: kenxu95 <[email protected]> commit 86269ec Author: kenxu95 <[email protected]> Date: Thu May 11 12:09:42 2023 -0700 Add appropriate workflow count to our connected notification cards and details page (#1302) commit 7c75ad6 Author: kenxu95 <[email protected]> Date: Thu May 11 10:45:52 2023 -0700 Update the artifact storage card presentation to be consistent with the other data integrations (#1297) commit 4f9c227 Author: Andre Giron <[email protected]> Date: Wed May 10 16:55:23 2023 -0700 ENG_2960: Fix broken documentation link for Aqueduct demo resource. (#1306) Co-authored-by: Andre Giron <[email protected]> commit 6341917 Author: Andre Giron <[email protected]> Date: Wed May 10 16:28:23 2023 -0700 Release version 0.3.2 (#1307) Co-authored-by: Vikram Sreekanti <[email protected]> Co-authored-by: Andre Giron <[email protected]>

kenxu95 requested review from vsreekanti, agiron123 and likawind May 11, 2023 23:02

agiron123 reviewed May 11, 2023

View reviewed changes

kenxu95 force-pushed the eng-2852-add-entry-to-aqueduct-server-integration branch from 09aa29d to 3408cfb Compare May 11, 2023 23:44

kenxu95 requested a review from agiron123 May 11, 2023 23:46

likawind approved these changes May 15, 2023

View reviewed changes

agiron123 reviewed May 15, 2023

View reviewed changes

agiron123 approved these changes May 15, 2023

View reviewed changes

kenxu95 added 10 commits May 15, 2023 14:36

appears that I've done it

d1a7466

not sure about deletion

7fe0b78

the conda details stuff works!

99a0ed3

context switch

24b8012

ready for preliminary review

0245c65

after code reivew

4d00b63

rebased

16336ba

fixed the filesystem card overrun

dedd082

cr

d7263d4

rebased onto last and ready to merge

e235a5e

kenxu95 force-pushed the eng-2852-add-entry-to-aqueduct-server-integration branch from 16ac466 to e235a5e Compare May 15, 2023 21:56

kenxu95 added the run_integration_test Triggers integration tests label May 15, 2023

kenxu95 merged commit 942dec1 into main May 15, 2023

kenxu95 deleted the eng-2852-add-entry-to-aqueduct-server-integration branch May 15, 2023 22:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[UI] Merge the Conda resource into the Aqueduct Server #1311

[UI] Merge the Conda resource into the Aqueduct Server #1311

kenxu95 commented May 11, 2023 •

edited

Loading

agiron123 May 11, 2023

likawind left a comment

likawind May 12, 2023

kenxu95 May 15, 2023

likawind May 15, 2023

kenxu95 May 15, 2023

likawind left a comment

agiron123 May 15, 2023

kenxu95 commented May 15, 2023

[UI] Merge the Conda resource into the Aqueduct Server #1311

[UI] Merge the Conda resource into the Aqueduct Server #1311

Conversation

kenxu95 commented May 11, 2023 • edited Loading

Describe your changes and why you are making these changes

Related issue number (if any)

Loom demo (if any)

Checklist before requesting a review

agiron123 May 11, 2023

Choose a reason for hiding this comment

likawind left a comment

Choose a reason for hiding this comment

likawind May 12, 2023

Choose a reason for hiding this comment

kenxu95 May 15, 2023

Choose a reason for hiding this comment

likawind May 15, 2023

Choose a reason for hiding this comment

kenxu95 May 15, 2023

Choose a reason for hiding this comment

likawind left a comment

Choose a reason for hiding this comment

agiron123 May 15, 2023

Choose a reason for hiding this comment

kenxu95 commented May 15, 2023

kenxu95 commented May 11, 2023 •

edited

Loading