Skip to content

[Improvement] Fix a potential problem when editing an existing mysql data source#5670

Merged
CalvinKirs merged 1 commit intoapache:devfrom
echohlne:edit_existsed_datasource_error_null
Jun 21, 2021
Merged

[Improvement] Fix a potential problem when editing an existing mysql data source#5670
CalvinKirs merged 1 commit intoapache:devfrom
echohlne:edit_existsed_datasource_error_null

Conversation

@echohlne
Copy link
Contributor

Purpose of the pull request

Fix a potential problem that editing an existing mysql data source may cause the backend to return null data.
reproduce step:

  1. Create a normal MySQL data source
  2. Modify the IP of the data source created in the step 1, by modify ip to a non-existent IP, and then directly click the edit button (skip the test connection), then we'll seen an error message tip with no content

image (2)

Request URL: http://localhost:8888/dolphinscheduler/datasources/update
response {"code":null,"msg":null,"data":null}

Brief change log

fix a special case in edit an existing datasource.

Result<Object> isConnection = checkConnection(dataSource.getType(), connectionParam);
        if (Status.SUCCESS.getCode() != isConnection.getCode()) {
            return result;
        }

The result instance does not have a certain code and msg value, which results in the content returned to the front end shows as null. In this case, just return isConnection directly.

Verify this pull request

Using Existed UTs.
Alse munutal test works well

Copy link
Member

@CalvinKirs CalvinKirs left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

Codecov Report

Merging #5670 (66b8cfa) into dev (a768214) will increase coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##                dev    #5670   +/-   ##
=========================================
  Coverage     45.31%   45.32%           
+ Complexity     3673     3672    -1     
=========================================
  Files           607      607           
  Lines         24794    24794           
  Branches       2803     2803           
=========================================
+ Hits          11235    11237    +2     
+ Misses        12489    12486    -3     
- Partials       1070     1071    +1     
Impacted Files Coverage Δ
...eduler/api/service/impl/DataSourceServiceImpl.java 44.76% <0.00%> (ø)
...dolphinscheduler/remote/future/ResponseFuture.java 81.35% <0.00%> (-1.70%) ⬇️
...er/master/processor/queue/TaskResponseService.java 68.49% <0.00%> (+4.10%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a768214...66b8cfa. Read the comment docs.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@CalvinKirs CalvinKirs merged commit e2eed1f into apache:dev Jun 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants