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

✨ Announcements entrypoint at the web-api (⚠️ devops) #4487

Merged
merged 21 commits into from
Jul 12, 2023

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Jul 11, 2023

What do these changes do?

New plugin to handle public announcements:

Related issue/s

How to test

  • driving test
cd services/web/server
pytest tests/unit/with_dbs/02/test_announcements.py

DevOps

  • ⚠️ web-server (i.e. not wb-db-event-listener or.env-wb-db-event-listener) must be configured with WEBSERVER_ANNOUNCEMENTS=1
  • ⚠️ Activate in redis commander new db: announcements:${REDIS_HOST}:${REDIS_PORT}:5
  • Request for ideas: GET /announcements entrypoint is opened
    • plugin can be enabled/disabled with envvar WEBSERVER_ANNOUNCEMENTS: bool
    • limit rate to this entrypoint (traeffik!) ?
    • Any extra suggestion?

@pcrespov pcrespov self-assigned this Jul 11, 2023
@pcrespov pcrespov added the a:webserver issue related to the webserver service label Jul 11, 2023
@pcrespov pcrespov added this to the Watermelon milestone Jul 11, 2023
@pcrespov pcrespov force-pushed the is21/announcements-webapi branch from a7a5748 to 6161026 Compare July 11, 2023 12:29
@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

Merging #4487 (256434b) into master (eb5e00b) will increase coverage by 0.0%.
The diff coverage is 97.9%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #4487   +/-   ##
======================================
  Coverage    86.4%   86.4%           
======================================
  Files        1013    1018    +5     
  Lines       43218   43305   +87     
  Branches     1006    1006           
======================================
+ Hits        37370   37455   +85     
- Misses       5616    5618    +2     
  Partials      232     232           
Flag Coverage Δ
integrationtests 67.9% <63.2%> (-0.1%) ⬇️
unittests 84.1% <97.9%> (+<0.1%) ⬆️

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

Impacted Files Coverage Δ
...ges/settings-library/src/settings_library/redis.py 0.0% <0.0%> (ø)
.../web/server/src/simcore_service_webserver/redis.py 95.6% <92.8%> (+0.1%) ⬆️
...rc/simcore_service_webserver/announcements/_api.py 100.0% <100.0%> (ø)
...mcore_service_webserver/announcements/_handlers.py 100.0% <100.0%> (ø)
...simcore_service_webserver/announcements/_models.py 100.0% <100.0%> (ø)
.../simcore_service_webserver/announcements/_redis.py 100.0% <100.0%> (ø)
.../simcore_service_webserver/announcements/plugin.py 100.0% <100.0%> (ø)
...erver/src/simcore_service_webserver/application.py 98.0% <100.0%> (+<0.1%) ⬆️
.../simcore_service_webserver/application_settings.py 98.2% <100.0%> (+<0.1%) ⬆️

... and 1 file with indirect coverage changes

@pcrespov pcrespov changed the title WIP: ✨ Is21/announcements webapi WIP: ✨ Is21/announcements webapi (⚠️ devops) Jul 11, 2023
@pcrespov pcrespov changed the title WIP: ✨ Is21/announcements webapi (⚠️ devops) ✨ Is21/announcements webapi (⚠️ devops) Jul 11, 2023
@pcrespov pcrespov marked this pull request as ready for review July 11, 2023 14:30
@pcrespov pcrespov force-pushed the is21/announcements-webapi branch from 0d5172f to 46e8c68 Compare July 11, 2023 14:31
@pcrespov pcrespov requested review from odeimaiz and YuryHrytsuk July 11, 2023 14:31
@pcrespov pcrespov changed the title ✨ Is21/announcements webapi (⚠️ devops) ✨ Announcements entrypoint at the web-api (⚠️ devops) Jul 11, 2023
Copy link
Member

@mrnicegyu11 mrnicegyu11 left a comment

Choose a reason for hiding this comment

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

Thanks a lot very nice, just some thoughts from my side for this one :)

DevOps tasks are acknowledged, Currently the webserver gets almost all routing and it would be more than just some minutes work to implement a special rate-limit for this specific route. The announcements will be rate-limited just as all other API of the webserver, which was raised for the TIP deployment but is still set at some reasonable values.

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.

Thanks a lot!

I believe you miss increasing the --databases to 6 in services/docker-compose.yml

@mrnicegyu11
Copy link
Member

@pcrespov your PR should propagate together with ITISFoundation/osparc-ops-environments#272 that includes the devops changes for the redis-commander

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.

green from me 👍

@pcrespov pcrespov requested a review from odeimaiz July 11, 2023 15:49
@pcrespov pcrespov force-pushed the is21/announcements-webapi branch from 196bfea to 80bfa11 Compare July 11, 2023 15:50
@pcrespov pcrespov force-pushed the is21/announcements-webapi branch from 80bfa11 to e3293a4 Compare July 12, 2023 09:47
@pcrespov pcrespov requested a review from GitHK July 12, 2023 10:44
@pcrespov pcrespov enabled auto-merge (squash) July 12, 2023 10:44
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.

Tested and working as expected. Thanks!!

@codeclimate
Copy link

codeclimate bot commented Jul 12, 2023

Code Climate has analyzed commit 256434b and detected 0 issues on this pull request.

View more on Code Climate.

@sonarcloud
Copy link

sonarcloud bot commented Jul 12, 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

@pcrespov pcrespov disabled auto-merge July 12, 2023 12:17
@pcrespov pcrespov merged commit 136ca89 into ITISFoundation:master Jul 12, 2023
@pcrespov pcrespov deleted the is21/announcements-webapi branch July 12, 2023 12:17
pcrespov added a commit that referenced this pull request Jul 13, 2023
mrnicegyu11 pushed a commit to ITISFoundation/osparc-ops-environments that referenced this pull request Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:webserver issue related to the webserver service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants