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

[DSIP-39][parameter] Startup parameters/global parameters/project parameters support set data type #15936

Closed
3 tasks done
Tracked by #14102
sdhzwc opened this issue Apr 29, 2024 · 9 comments · Fixed by #15967
Closed
3 tasks done
Tracked by #14102
Assignees
Labels
DSIP feature new feature improvement make more easy to user or prompt friendly priority:middle

Comments

@sdhzwc
Copy link
Contributor

sdhzwc commented Apr 29, 2024

Search before asking

  • I had searched in the issues and found no similar feature requirement.

Motivation

Currently startup parameters, global parameters, project parameters do not support custom data value types, the default is VARCHAR data value type, if I want to use the data type is LONG, this time there is no way to carry out the operation, so this optimization point is to give more choice points to the user, to increase the user experience.

Design Detail

一、 DESC:
This PR is for the startup parameter, global parameter, project parameter support custom data value type, to meet our needs for data use, increase user experience. Which I did not modify the logic of the big, just load the data value type to personalize the limited operation, specifically the following points:

  • 1.1 the default data type of startup parameters, global parameters, project parameters is VARCHAR, then this time I transformed this, support for custom data types and compatible with the original data type operation.
  • 1.2 StartParams was originally MAP way, now changed to LIST, which I judged whether it is JSON compatible operation.
  • 1.3 add New UI
  • 1.4 add UT

二、 EFFECT DISPLAY:
2.1 GIF
111111111111111111-32-34
2.2 Startup Parameters
启动参数
2.3 Global Parameters
全局参数
2.4 Project Parameters
项目参数

Compatibility, Deprecation, and Migration Plan

Compatibility with latest version.

Test Plan

Test by UT.

Are you willing to submit a PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@sdhzwc sdhzwc added improvement make more easy to user or prompt friendly Waiting for reply Waiting for reply labels Apr 29, 2024
@ruanwenjun ruanwenjun removed the Waiting for reply Waiting for reply label Apr 29, 2024
@ruanwenjun
Copy link
Member

Good idea, do you have any designs?

@sdhzwc
Copy link
Contributor Author

sdhzwc commented Apr 30, 2024

Good idea, do you have any designs?

1, the first step in the database to increase param_data_type field default value is VARCHAR
2, the second step to modify the add, modify, query logic
3, the third step is to modify the corresponding loading logic
4, and finally increase the UI and UT

@ruanwenjun
Copy link
Member

Good idea, do you have any designs?

1, the first step in the database to increase param_data_type field default value is VARCHAR 2, the second step to modify the add, modify, query logic 3, the third step is to modify the corresponding loading logic 4, and finally increase the UI and UT

The design is too simple, since this is a new feature, please change the issue to DSIP and provide the detail design, then we can discuss the design.

Startup parameters/global parameters/task parameters also need to have types, otherwise, this will be very strange and think of as a bug.

@sdhzwc
Copy link
Contributor Author

sdhzwc commented May 6, 2024

Good idea, do you have any designs?

1, the first step in the database to increase param_data_type field default value is VARCHAR 2, the second step to modify the add, modify, query logic 3, the third step is to modify the corresponding loading logic 4, and finally increase the UI and UT

The design is too simple, since this is a new feature, please change the issue to DSIP and provide the detail design, then we can discuss the design.

Startup parameters/global parameters/task parameters also need to have types, otherwise, this will be very strange and think of as a bug.

In fact, this is not a new feature, the parameter priority and loading order is not changed, just optimize the type of parameter values, from the default VARCHAR to now custom type

@ruanwenjun
Copy link
Member

Good idea, do you have any designs?

1, the first step in the database to increase param_data_type field default value is VARCHAR 2, the second step to modify the add, modify, query logic 3, the third step is to modify the corresponding loading logic 4, and finally increase the UI and UT

The design is too simple, since this is a new feature, please change the issue to DSIP and provide the detail design, then we can discuss the design.
Startup parameters/global parameters/task parameters also need to have types, otherwise, this will be very strange and think of as a bug.

In fact, this is not a new feature, the parameter priority and loading order is not changed, just optimize the type of parameter values, from the default VARCHAR to now custom type

Right now, we don't support types in DS project params/workflow params/ startup params, this issue is want to support this, so this is a new feature.

You changed the API interface, this is an incompatible change, but it doesn't announce in the issue/PR. The worst thing that can happen is that the user cp's this PR and the original interface will be unusable.

This is why I request a DSIP.

In the description and PR, I didn't see there exists a validation step, this means I can create a parameter with the type int and value tom? This is unacceptable.

@sdhzwc
Copy link
Contributor Author

sdhzwc commented May 6, 2024

Good idea, do you have any designs?

1, the first step in the database to increase param_data_type field default value is VARCHAR 2, the second step to modify the add, modify, query logic 3, the third step is to modify the corresponding loading logic 4, and finally increase the UI and UT

The design is too simple, since this is a new feature, please change the issue to DSIP and provide the detail design, then we can discuss the design.
Startup parameters/global parameters/task parameters also need to have types, otherwise, this will be very strange and think of as a bug.

In fact, this is not a new feature, the parameter priority and loading order is not changed, just optimize the type of parameter values, from the default VARCHAR to now custom type

Right now, we don't support types in DS project params/workflow params/ startup params, this issue is want to support this, so this is a new feature.

You changed the API interface, this is an incompatible change, but it doesn't announce in the issue/PR. The worst thing that can happen is that the user cp's this PR and the original interface will be unusable.

This is why I request a DSIP.

In the description and PR, I didn't see there exists a validation step, this means I can create a parameter with the type int and value tom? This is unacceptable.

I understand what you mean, in fact, the new data value field, does not affect its original logic, because the default value of the field in the database is VARCHAR, in fact, has done a compatible operation!
In fact, I feel that there is a point that really need to be optimized, that is, the global parameters, start parameters, if only the project parameters, custom parameters are indeed a little strange!

@ruanwenjun
Copy link
Member

@sdhzwc It's OK for me to support the global params/ start parameters in different PR(it's needed to create sub-issues for this).

I see in the PR, this interface
image has break the competition. DSIP means this PR introduces a big change. All changes to the API and table structure are considered as big changes

@sdhzwc sdhzwc changed the title [Improvement][parameter] New data types and type filtering [DSIP][parameter] New data types and type filtering May 8, 2024
@sdhzwc sdhzwc changed the title [DSIP][parameter] New data types and type filtering [DSIP][parameter] Improvement startup parameters/global parameters/project parameters data type May 8, 2024
@sdhzwc
Copy link
Contributor Author

sdhzwc commented May 8, 2024

一、 DESC:
This PR is for the startup parameter, global parameter, project parameter support custom data value type, to meet our needs for data use, increase user experience. Which I did not modify the logic of the big, just load the data value type to personalize the limited operation, specifically the following points:
1, the default data type of startup parameters, global parameters, project parameters is VARCHAR, then this time I transformed this, support for custom data types and compatible with the original data type operation.
2, StartParams was originally MAP way, now changed to LIST, which I judged whether it is JSON compatible operation.
3、add New UI
4, add UT

At present is my optimization of the function of the operation, with the design of the language I may not be able to express the situation, where insufficient can be listed, I will be improved

二、 EFFECT DISPLAY:
2.1 GIF
111111111111111111-32-34
2.2 Startup Parameters
启动参数
2.3 Global Parameters
全局参数
2.4 Project Parameters
项目参数

@ruanwenjun I've changed it to DSIP and you can see where else it doesn't fit.

@ruanwenjun ruanwenjun changed the title [DSIP][parameter] Improvement startup parameters/global parameters/project parameters data type [DSIP-39][parameter] Startup parameters/global parameters/project parameters support set data type May 9, 2024
@ruanwenjun
Copy link
Member

ruanwenjun commented May 9, 2024

@sdhzwc Great, basically LGTM, I have add this issue to DSIP list, please change the issue description, you can refer #15931, DSIP should contains Motivation, Design Detail, Compatibility, Deprecation, and Migration Plan, Test Plan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DSIP feature new feature improvement make more easy to user or prompt friendly priority:middle
Projects
None yet
2 participants