Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

added data connection section #1000

Merged
merged 7 commits into from Mar 24, 2023
Merged

added data connection section #1000

merged 7 commits into from Mar 24, 2023

Conversation

Gkrumbach07
Copy link
Member

@Gkrumbach07 Gkrumbach07 commented Mar 10, 2023 •
edited

closes #779

Description

Created a new data connection section

How Has This Been Tested?

Test that you can:

  1. create a new workspace with a new data connection
  2. create a new workspace with an existing data connection
  3. edit a workspace with a new data connection
  4. edit a workspace with an existing data connection

only one data connection per workspace. but multiple workspaces can share a connection

Test Impact

Request review criteria:

image

image

  • The commits have meaningful messages (squashes happen on merge by the bot).
  • Included any necessary screenshots or gifs if it was a UI change.
  • Included tags to the UX team if it was a UI/UX change.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work
  • The developer has added tests or explained why testing cannot be added (unit tests & storybook for related changes)

@andrewballantyne
Copy link
Member

/retitle [WIP] added data connection section

Missing Upload Secret, this is still a work in progress.

@openshift-ci openshift-ci bot changed the title added data connection section [WIP] added data connection section Mar 10, 2023
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress This PR is in WIP state label Mar 10, 2023
@Gkrumbach07
added data connection section
a31db6d 
added upload for secret
@Gkrumbach07 Gkrumbach07 changed the title [WIP] added data connection section added data connection section Mar 15, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress This PR is in WIP state label Mar 15, 2023
lucferbux
lucferbux approved these changes Mar 23, 2023
View reviewed changes
Copy link
Contributor

@lucferbux lucferbux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Mar 23, 2023
andrewballantyne
andrewballantyne requested changes Mar 24, 2023
View reviewed changes
Copy link
Member

@andrewballantyne andrewballantyne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing the UI now...

frontend/src/pages/projects/screens/spawner/dataConnection/useNotebookDataConnection.ts Outdated Show resolved Hide resolved
@openshift-ci openshift-ci bot removed the lgtm label Mar 24, 2023
@andrewballantyne
Copy link
Member

It would seem that if you type the same name as an existing data connection it will create the PVC and then fail -- you correct the error and the PVC is already created. We may need dry runs here.

andrewballantyne
andrewballantyne approved these changes Mar 24, 2023
View reviewed changes
Copy link
Member

@andrewballantyne andrewballantyne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll have to swing back through and improve the footer and get better saving in place. But this should be good for now.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 24, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewballantyne, lucferbux

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the lgtm label Mar 24, 2023
@openshift-merge-robot openshift-merge-robot merged commit 32f4555 into opendatahub-io:main Mar 24, 2023
2 checks passed
@Gkrumbach07
Copy link
Member Author

We'll have to swing back through and improve the footer and get better saving in place. But this should be good for now.

#1042

bartoszmajsak pushed a commit to maistra/odh-dashboard that referenced this pull request Mar 30, 2023
@Gkrumbach07 @bartoszmajsak
added data connection section (opendatahub-io#1000)
9f1646f 
* added data connection section

added upload for secret

* removed extra PF props

* fixed props

* added data connection form validation

* fixes

* added fix

* linter
@Gkrumbach07 Gkrumbach07 deleted the data-connections branch August 10, 2023 15:33
strangiato pushed a commit to strangiato/odh-dashboard that referenced this pull request Oct 18, 2023
@Gkrumbach07 @strangiato
added data connection section (opendatahub-io#1000)
a1a12bb 
* added data connection section

added upload for secret

* removed extra PF props

* fixed props

* added data connection form validation

* fixes

* added fix

* linter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DaoDaoNoCode DaoDaoNoCode DaoDaoNoCode requested changes

@andrewballantyne andrewballantyne andrewballantyne approved these changes

@lucferbux lucferbux lucferbux approved these changes

Assignees

@andrewballantyne andrewballantyne

@lucferbux lucferbux

@DaoDaoNoCode DaoDaoNoCode

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[DSG]: Data Connections are hard to find on Workbench Creation
5 participants
@Gkrumbach07 @andrewballantyne @lucferbux @DaoDaoNoCode @openshift-merge-robot