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

♻️ Refactors projects plugin sub-modules as controller-service-repository #4389

Merged
merged 17 commits into from
Jun 22, 2023

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Jun 21, 2023

What do these changes do?

Reorganizing webserver's projects plugin to reflect controller-service-repository pattern.
image

For instance, let's assume a project's sub-resource foo. It should be split in three submodules as

  • projects/_foo_handlers.py : controler
  • projects/_foo_api.py: service
  • projects/_foo_db.py: repository

This PR adapts only some of the submodules, namely:

  • projects/ports
  • projects/permalinks
  • projects/nodes
  • projects/tags
  • deprecated nodes submodule was removed. i.e. they are not used.

Related issue/s

How to test

In-place

DevOps

None

@pcrespov pcrespov self-assigned this Jun 21, 2023
@pcrespov pcrespov changed the title Maintenance/projects layout ♻️ Refactors projects plugin sub-modules as controller-service-repository Jun 21, 2023
@pcrespov pcrespov added the t:maintenance Some planned maintenance work label Jun 21, 2023
@pcrespov pcrespov added this to the Watermelon milestone Jun 21, 2023
@pcrespov pcrespov marked this pull request as ready for review June 21, 2023 08:38
@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Merging #4389 (269420e) into master (48d29cd) will decrease coverage by 1.6%.
The diff coverage is 100.0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #4389      +/-   ##
=========================================
- Coverage    86.1%   84.6%    -1.6%     
=========================================
  Files         987     394     -593     
  Lines       42403   20695   -21708     
  Branches     1006     173     -833     
=========================================
- Hits        36551   17511   -19040     
+ Misses       5621    3135    -2486     
+ Partials      231      49     -182     
Flag Coverage Δ
integrationtests 67.9% <62.5%> (+<0.1%) ⬆️
unittests 86.6% <100.0%> (+2.8%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...mcore_service_webserver/projects/_permalink_api.py 97.5% <ø> (ø)
...c/simcore_service_webserver/projects/_ports_api.py 93.9% <ø> (ø)
...mcore_service_webserver/projects/_tags_handlers.py 92.8% <ø> (ø)
...e_service_webserver/projects/_crud_create_utils.py 96.8% <100.0%> (ø)
...ore_service_webserver/projects/_crud_read_utils.py 97.5% <100.0%> (ø)
...mcore_service_webserver/projects/_handlers_crud.py 82.0% <100.0%> (ø)
...c/simcore_service_webserver/projects/_nodes_api.py 77.9% <100.0%> (ø)
...core_service_webserver/projects/_nodes_handlers.py 89.1% <100.0%> (ø)
...core_service_webserver/projects/_ports_handlers.py 94.3% <100.0%> (ø)
...imcore_service_webserver/projects/_rest_schemas.py 100.0% <100.0%> (ø)
... and 3 more

... and 669 files with indirect coverage changes

Copy link
Contributor

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

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

great! thanks 👍

Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

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

A few considerations:

  • I would personally prefer to use *_repository.py in place of *_db.py since: it can be Postregres, it can be Redis, it can be S3.
  • why are the nodes api endpoints deleted? I found nothing in the description of the PR

@pcrespov
Copy link
Member Author

pcrespov commented Jun 21, 2023

  • I would personally prefer to use *_repository.py in place of *_db.py since: it can be Postregres, it can be Redis, it can be S3.
  • IMO db is good enough (discussed with @matusdrobuliak66), otherwise the names become far too long. Same way that we do not write service but api.
  • why are the nodes api endpoints deleted? I found nothing in the description of the PR
  • They were deprecated (it was actually written in the description but now i have highlighted it). We never used them.

@pcrespov pcrespov force-pushed the maintenance/projects-layout branch from b03711f to ec41a5c Compare June 21, 2023 14:59
@pcrespov pcrespov requested a review from GitHK June 21, 2023 15:21
@pcrespov pcrespov force-pushed the maintenance/projects-layout branch from ec41a5c to aa1ca3b Compare June 21, 2023 15:22
@sonarcloud
Copy link

sonarcloud bot commented Jun 21, 2023

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
0.0% 0.0% Duplication

@codeclimate
Copy link

codeclimate bot commented Jun 21, 2023

Code Climate has analyzed commit 269420e and detected 0 issues on this pull request.

View more on Code Climate.

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

ok. if possible, wait for my PR about node resources to go in before pushing this one.

@pcrespov pcrespov merged commit f61f6ea into ITISFoundation:master Jun 22, 2023
@pcrespov pcrespov deleted the maintenance/projects-layout branch June 22, 2023 08:20
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Jul 12, 2023
36 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t:maintenance Some planned maintenance work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants