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.

Sign up for GitHub

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

Jump to bottom

Is765/fix default product (⚠️ devops ) #3552

Merged
merged 19 commits into from
Nov 16, 2022
Merged

Is765/fix default product (⚠️ devops ) #3552

merged 19 commits into from
Nov 16, 2022

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Nov 13, 2022 •
edited

What do these changes do?

The design of products was originally conceived as a way to serve different "software products" (e.g. different front-end, catalog, ... ) within the same deployment in addition to the default osparc product. Nonetheless, this idea has evolved into having a single product per deploy which might NOT necessarily be osparc.

In this PR we default the concept of default_product which corresponds to the item with highest priority in the products table.

This definition affects three services, namely the web-server, the catalog and the api-server.

⚠️ devops

  • removed API_SERVER_DEFAULT_PRODUCT_NAME
  • removed CATALOG_ACCESS_RIGHTS_DEFAULT_PRODUCT_NAME
  • ENSURE that products.priority==0 for the main product in the deploy and and products.priority>0 for the rest
  • save new products table in https://git.speag.com/oSparc/products

Related issue/s

How to test

pytest -vv tests/**/test_product*.py

@pcrespov pcrespov self-assigned this Nov 13, 2022
@codecov
Copy link

codecov bot commented Nov 13, 2022 •
edited

Codecov Report

Merging #3552 (f8bce03) into master (f8bce03) will not change coverage.
The diff coverage is n/a.

❗ Current head f8bce03 differs from pull request most recent head 6f6407d. Consider uploading reports for the commit 6f6407d to get more accurate results

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #3552   +/-   ##
======================================
  Coverage    83.4%   83.4%           
======================================
  Files         854     854           
  Lines       35945   35945           
  Branches      765     765           
======================================
  Hits        30002   30002           
  Misses       5744    5744           
  Partials      199     199
Flag Coverage Δ
integrationtests 67.0% <0.0%> (ø)
unittests 80.6% <0.0%> (ø)

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

@pcrespov pcrespov added this to the Athena milestone Nov 13, 2022
@pcrespov pcrespov added a:webserver issue related to the webserver service a:catalog catalog service a:apiserver api-server service labels Nov 15, 2022
@pcrespov pcrespov changed the title WIP: Is765/fix default product Is765/fix default product Nov 15, 2022
@pcrespov pcrespov marked this pull request as ready for review November 15, 2022 16:01
@pcrespov pcrespov changed the title Is765/fix default product Is765/fix default product (⚠️ devops ) Nov 15, 2022
sanderegg
sanderegg approved these changes Nov 15, 2022
View reviewed changes
packages/postgres-database/src/simcore_postgres_database/utils_products.py

:: raises ValueError if undefined
"""
product_name = await conn.scalar(
Copy link
Member

Choose a reason for hiding this comment

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

so the obvious question.. who creates the entry in the database?

Copy link
Member Author

Choose a reason for hiding this comment

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

It has to be done by an operator

Copy link
Member Author

@pcrespov pcrespov Nov 15, 2022 •
edited

Choose a reason for hiding this comment

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

But if it is not done, the services fail to start

services/api-server/tests/unit/conftest.py
@@ -101,7 +101,7 @@ def auth(mocker, app: FastAPI, faker: Faker) -> HTTPBasicAuth:
# mock engine if db was not init
if app.state.settings.API_SERVER_POSTGRES is None:

engine = mocker.Mock()
engine = mocker.MagicMock()
Copy link
Member

Choose a reason for hiding this comment

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

Q: what is the difference?

Copy link
Member Author

@pcrespov pcrespov Nov 15, 2022 •
edited

Choose a reason for hiding this comment

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

one is magic, the other isn't ;-b

e.g. in this case, with Magic he auto-generate mocks for functions e.g. async with engine.acquire() would return a new MagicMock. This does not happen with only Mock

@pcrespov pcrespov enabled auto-merge (squash) November 15, 2022 19:13
GitHK
GitHK reviewed Nov 16, 2022
View reviewed changes
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.

Some comments below

services/web/server/src/simcore_service_webserver/products_events.py Outdated Show resolved Hide resolved
packages/postgres-database/src/simcore_postgres_database/utils_products.py Show resolved Hide resolved
@pcrespov pcrespov requested a review from GitHK November 16, 2022 07:54
GitHK
GitHK approved these changes Nov 16, 2022
View reviewed changes
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.

👍 Very nice! Thanks for the changes.

@pcrespov pcrespov enabled auto-merge (squash) November 16, 2022 09:57
odeimaiz
odeimaiz approved these changes Nov 16, 2022
View reviewed changes
Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

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

👍

@sonarcloud
Copy link

sonarcloud bot commented Nov 16, 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
0.0% 0.0% Duplication

@pcrespov pcrespov merged commit 163e216 into ITISFoundation:master Nov 16, 2022
@pcrespov pcrespov deleted the is765/fix-default-product branch November 16, 2022 15:05
@sanderegg sanderegg mentioned this pull request Dec 1, 2022
Closed
@pcrespov pcrespov mentioned this pull request Dec 16, 2022
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GitHK GitHK GitHK approved these changes

@odeimaiz odeimaiz odeimaiz approved these changes

@sanderegg sanderegg sanderegg approved these changes

@mrnicegyu11 mrnicegyu11 Awaiting requested review from mrnicegyu11

@Surfict Surfict Awaiting requested review from Surfict

Assignees

@pcrespov pcrespov

Labels
a:apiserver api-server service a:catalog catalog service a:webserver issue related to the webserver service
Projects
None yet
Milestone
Athena
Development

Successfully merging this pull request may close these issues.

Catalog cannot handle deployment with only tis product
4 participants
@pcrespov @GitHK @odeimaiz @sanderegg