-
Notifications
You must be signed in to change notification settings - Fork 167
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.
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
Make data connection bucket field mandatory #1949
Make data connection bucket field mandatory #1949
Conversation
@vconzola Can you check this very tiny change? I am not sure if it's necessary but it is a UI change, just add a small |
3c8adab
to
18c59ea
Compare
@DaoDaoNoCode This is just for model serving, the goal is that in the main form Bucket is not mandatory but in model serving is required. The rest looks good, great testuite. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isRequired should only be mandatory in model serving
Maybe im wrong here but let me know @andrewballantyne @DaoDaoNoCode is that right? |
@lucferbux I confirmed with @andrewballantyne, you are right, it's only required here but not everywhere. |
18c59ea
to
d760850
Compare
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: 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 |
@DaoDaoNoCode LGTM. Thanks for asking. ;-) |
/lgtm |
a9b3aae
into
opendatahub-io:f/model-serving
Closes #1583
Description
Simply made the bucket field required.
How Has This Been Tested?
New data connection
, make sure theBucket
field is requiredTest Impact
Add some unit tests for
isAWSValid
functionRequest review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
main