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.

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

Validate hostPath based on the error callback from FileAutoComplete #1481

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jelly
Copy link
Member

@jelly jelly commented Nov 10, 2023

This depends on this PR in Cockpit: cockpit-project/cockpit#19600

@martinpitt you looked at the validation PR before, mind giving this a look as well?

@@ -659,6 +659,7 @@ export class ImageRunModal extends React.Component {
* Index needs to corellate with a row number
*/
dynamicListOnValidationChange = (value, key) => {
console.log('dynamic list validation', value, key);
Copy link
Member Author

Choose a reason for hiding this comment

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

debug leftover....

src/Volume.jsx Outdated
@@ -38,9 +40,11 @@ export const Volume = ({ id, item, onChange, idx, removeitem, additem, options,
>
<FileAutoComplete id={id + "-host-path"}
value={item.hostPath || ''}
onChange={value => {
onChange={(value, error) => {
console.log(`value: ${value}, error: ${error}`);
Copy link
Member Author

Choose a reason for hiding this comment

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

debug leftover

@martinpitt
Copy link
Member

Okay, can you please clean up the debugging leftovers, and add an integration test? Thanks!

@jelly
Copy link
Member Author

jelly commented Nov 13, 2023

Okay, can you please clean up the debugging leftovers, and add an integration test? Thanks!

How do we deal with the dependent Cockpit PR?

Edit: Landed cockpit-project/cockpit#19600 so will do the follow up.

@jelly jelly force-pushed the volume-host-path-validation branch from 40d33d0 to 297b0ee Compare November 13, 2023 12:54
@jelly
Copy link
Member Author

jelly commented Nov 13, 2023

Okay, can you please clean up the debugging leftovers, and add an integration test? Thanks!

Done, but now I found another issue. When we provide a path without a trailing slash the validation doesn't work.

@jelly jelly force-pushed the volume-host-path-validation branch from 297b0ee to 96b1cf2 Compare November 13, 2023 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants