Skip to content
This repository has been archived by the owner on Jun 6, 2024. It is now read-only.

Binyli/bug fix #3134

Merged
merged 3 commits into from
Jul 10, 2019
Merged

Binyli/bug fix #3134

merged 3 commits into from
Jul 10, 2019

Conversation

Binyang2014
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 51.225% when pulling 9fa6a87 on binyli/bug-fix into f8e2d92 on pai-0.14.y.

@coveralls
Copy link

coveralls commented Jul 9, 2019

Coverage Status

Coverage increased (+0.3%) to 51.565% when pulling 5673a63 on binyli/bug-fix into f8e2d92 on pai-0.14.y.

@@ -187,14 +187,14 @@ export const DockerSection = ({onValueChange, value}) => {
if (optionKey !== 'customize-image' && isCutomizedImageEnabled) {
setCutomizedImageEnabled(false);
}
}, []);
}, [uri]);
Copy link
Contributor

@sunqinzheng sunqinzheng Jul 9, 2019

Choose a reason for hiding this comment

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

I think the isCustomizedImageEnabled state should be determined by uri prop.
You can use a useMemo hook to store the value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

isCustomizedImageEnabled is not just depends on the uri pop. Customers can also change its' value. So this situation could happens: uri not changed but isCustomizedImageEnabled is changed by customer.
Maybe use state is better for this

Copy link
Contributor

Choose a reason for hiding this comment

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

useEffect will be triggered each time page is rerendered. So if user input an existing docker option in the custom docker url text field like ufoym/deepo:tensorflow-py36-cu100, this hook will be triggered and isCutomizedImageEnabled will be set to false.

@sunqinzheng
Copy link
Contributor

By the way, the ufoym/deepo:all key in previous mapping function getDockerImageOptionKey is not matched with options object.

@Binyang2014
Copy link
Contributor Author

By the way, the ufoym/deepo:all key in previous mapping function getDockerImageOptionKey is not matched with options object.

Thanks for this, notice that

@sunqinzheng sunqinzheng requested a review from debuggy July 10, 2019 07:08
@Binyang2014 Binyang2014 merged commit 9fe23ee into pai-0.14.y Jul 10, 2019
@Binyang2014 Binyang2014 deleted the binyli/bug-fix branch July 10, 2019 09:11
debuggy pushed a commit that referenced this pull request Jul 19, 2019
* fix bug: submit button is disabled after import validate yaml & docker image custmoized Toggle empty the commands

* minor update

* address comments
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants