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

[python] Refactor structure to avoid cycle import #11167

Merged
merged 1 commit into from
Jul 28, 2022

Conversation

zhongjiajie
Copy link
Member

Currently, our core and side module dependent on each others. and will cause
cycle import in our codebase, especially in issue #10905, we try to refactor
to solve this problem.

This patch do the following change:

  • Rename module side to models
  • Move core/base and core/sidebase to dir modules
  • Move configuration and default_config.yaml to the root of pydolphinscheduler

Currently, our core and side module dependent on each others. and will cause
cycle import in our codebase, especially in issue apache#10905, we try to refactor
to solve this problem.

This patch do the following change:

* Rename module `side` to `models`
* Move `core/base` and `core/sidebase` to dir `modules`
* Move `configuration` and `default_config.yaml` to the root of pydolphinscheduler
@zhongjiajie zhongjiajie self-assigned this Jul 27, 2022
@zhongjiajie zhongjiajie added improvement make more easy to user or prompt friendly Python labels Jul 27, 2022
@zhongjiajie zhongjiajie modified the milestones: 3.1.0-alpha, 3.0.1 Jul 27, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2022

Codecov Report

Merging #11167 (e35fc27) into dev (8b50578) will decrease coverage by 0.03%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##                dev   #11167      +/-   ##
============================================
- Coverage     40.26%   40.23%   -0.04%     
+ Complexity     4926     4925       -1     
============================================
  Files           982      982              
  Lines         37522    37558      +36     
  Branches       4124     4127       +3     
============================================
+ Hits          15110    15112       +2     
- Misses        20879    20911      +32     
- Partials       1533     1535       +2     
Impacted Files Coverage Δ
...erver/master/processor/queue/TaskEventService.java 75.00% <0.00%> (-5.36%) ⬇️
...che/dolphinscheduler/api/python/PythonGateway.java 19.81% <0.00%> (-1.84%) ⬇️
...inscheduler/api/service/impl/UsersServiceImpl.java 72.11% <0.00%> (-0.39%) ⬇️
...api/service/impl/ProcessDefinitionServiceImpl.java 31.73% <0.00%> (-0.13%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us.

@sonarcloud
Copy link

sonarcloud bot commented Jul 27, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

No Coverage information No Coverage information
No Duplication information No Duplication information

@zhongjiajie
Copy link
Member Author

PTAL @jieguangzhou @devosend @lyleshaw

@zhongjiajie
Copy link
Member Author

I think it can go now

Copy link
Contributor

@lyleshaw lyleshaw left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jieguangzhou jieguangzhou left a comment

Choose a reason for hiding this comment

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

LGTM

@zhongjiajie zhongjiajie merged commit 7a766cb into apache:dev Jul 28, 2022
@zhongjiajie zhongjiajie deleted the f-py-cycleh-import branch July 28, 2022 11:02
zhongjiajie added a commit that referenced this pull request Sep 17, 2022
Currently, our core and side module dependent on each others. and will cause
cycle import in our codebase, especially in issue #10905, we try to refactor
to solve this problem.

This patch do the following change:

* Rename module `side` to `models`
* Move `core/base` and `core/sidebase` to dir `modules`
* Move `configuration` and `default_config.yaml` to the root of pydolphinscheduler

(cherry picked from commit 7a766cb)
@zhongjiajie zhongjiajie added the release cherry-pick Mark this issue/PR had cherry-pick for release version label Sep 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement make more easy to user or prompt friendly Python release cherry-pick Mark this issue/PR had cherry-pick for release version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants