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

webportal: allow jsonc in job submission #2084

Merged

Conversation

lpaladin
Copy link
Contributor

Allowing user to import json with comment (JSONC) on job submission page.
Addressing #1986.

@msftclas
Copy link

msftclas commented Jan 25, 2019

CLA assistant check
All CLA requirements met.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 52.702% when pulling 9b451d9 on zhouhaoyu:zhouhy/webportal-allow-json-with-comments into 377eb2d on Microsoft:master.

@lpaladin lpaladin requested review from qfyin and sunqinzheng January 31, 2019 04:19
@sunqinzheng sunqinzheng requested a review from Gerhut January 31, 2019 06:11
@Gerhut Gerhut requested a review from fanyangCS January 31, 2019 06:42
@Gerhut
Copy link
Member

Gerhut commented Jan 31, 2019

Hi @zhouhaoyu , thanks for your contribution.

I just read #1986, and have not understand your expected experience yet.

To submit JSONC to PAI in vscode extension, there are two solutions:

  1. Convert JSONC to JSON (strip comments) in client side, and submit JSON to PAI.
  2. Make REST server able to receive JSONC as job config, also store JSONC for latter retrieving.

I assume you choose solution 1, I think these steps should be done 1 by 1:

  1. Make the extension support JSONC, strip comments in extension side.
  2. Propose that web portal should align to the vscode extension, to fill the expected experience:

    Export JSONC job config from vscode extension (beacuse JSONC will not stored in REST server) and import it to webportal for submission

(Is this your expected experience?)

As I know, vscode extension have not implement the JSONC support now. So directly making the step 2 done seems useless currently.

@fanyangCS @scarlett2018 for discussion.

@lpaladin
Copy link
Contributor Author

lpaladin commented Feb 1, 2019

To submit JSONC to PAI in vscode extension, there are two solutions:

  1. Convert JSONC to JSON (strip comments) in client side, and submit JSON to PAI.
  2. Make REST server able to receive JSONC as job config, also store JSONC for latter retrieving.

In fact, what I'm trying to achieve is allowing user to edit jsonc in vscode directly as the commented configuration file will be easier to read.

When user finishes their editing and wants to submit a job, they can either

  1. ...submit the file directly to web portal, without an extra step to strip the comments, which is currently not available due to web portal not allowing jsonc
  2. ...or use the extension to submit the configuration to PAI via the REST api directly, in which case the extension will take care of the conversion from jsonc to json

By allowing jsonc in web portal job submission page, the first option will be available.

@Gerhut
Copy link
Member

Gerhut commented Feb 13, 2019

Offline synced with @fanyangCS , this change could be checked-in as long as it introduces no extra dependencies. So approved, feel free to :shipit:

@lpaladin lpaladin merged commit 4d97a67 into microsoft:master Feb 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants