♻️Autoscaling: refactor before changes (⚠️ DEVOPS) #8002
Conversation
sanderegg
added
t:maintenance
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8002 +/- ##
==========================================
- Coverage 88.23% 88.11% -0.13%
==========================================
Files 1863 1856 -7
Lines 71964 71795 -169
Branches 1261 1261
==========================================
- Hits 63496 63260 -236
- Misses 8104 8171 +67
Partials 364 364
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
sanderegg
force-pushed
the
autoscaling/refactoring
branch
3 times, most recently
from
2cab411 to
3de8413
Compare
sanderegg
changed the title
sanderegg
requested review from
bisgaard-itis,
Copilot,
giancarloromeo and
matusdrobuliak66
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the autoscaling service by replacing legacy “buffer” naming with distinct “warm”/“hot” buffers, and by modularizing autoscaling modes into provider classes.
- Introduces
ComputationalAutoscalingProviderandDynamicAutoscalingProviderin place ofBaseAutoscalingsubclasses - Renames buffer-related utilities and model fields (
buffer→warm_buffer/hot_buffer) and updates EC2 tag helpers to operate on EC2Tags only - Updates imports and paths for cluster‐scaling, instrumentation, and tasks modules
Reviewed Changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| utils/warm_buffer_machines.py | Extracted buffer‐tag helpers; renamed and simplified tag functions |
| utils/utils_ec2.py | Added DNS parsing helpers; standardized logging and regex for private DNS |
| utils/utils_docker.py | Added helper to detect drained instances; simplified tag maps |
| utils/cluster_scaling.py | Renamed buffer functions to hot/warm; removed old DNS helpers |
| modules/cluster_scaling/_provider_dynamic.py | Converted static methods to instance providers |
| modules/cluster_scaling/_provider_computational.py | Converted static methods to instance providers |
| modules/cluster_scaling/_buffer_machines_pool_core.py | Switched buffer pool types to warm; updated tag usages |
| modules/cluster_scaling/_auto_scaling_core.py | Switched buffer pools to warm/hot; updated provider protocol usage |
| Numerous test files under tests/unit | Updated imports and renamed test fixtures to match hot/warm naming |
| core/application.py | Pointed background tasks setup to new cluster_scaling paths |
Comments suppressed due to low confidence (2)
services/autoscaling/src/simcore_service_autoscaling/modules/cluster_scaling/_buffer_machines_pool_core.py:309
- Wrap the tag value in AWSTagValue for consistency and validation (e.g. AWSTagValue("true")) rather than using a raw string literal.
BUFFER_MACHINE_PULLING_EC2_TAG_KEY: "true",
services/autoscaling/src/simcore_service_autoscaling/utils/utils_docker.py:280
- [nitpick] Consider extracting the reservation and limit values into helper variables or a small function to reduce nested parentheses and improve readability, e.g.:
res = task.spec.resources or Resources(); cpu_res = res.reservations.nano_cp_us or 0; cpu_lim = res.limits.nano_cp_us or 0; cpus = max(cpu_res, cpu_lim) / _NANO_CPU.
cpus=max(
1ef39e3 to
4dad4e7
Compare
|
@mergify queue |
🛑 The pull request has been removed from the queue
|
|
@mergify requeue |
☑️ This pull request is already queued |
sanderegg
force-pushed
the
autoscaling/refactoring
branch
from
4dad4e7 to
567266b
Compare
|
This pull request has been removed from the queue for the following reason: The merge conditions cannot be satisfied due to failing checks:You may have to fix your CI before adding the pull request to the queue again. |
sanderegg
force-pushed
the
autoscaling/refactoring
branch
from
30b7fe2 to
6083c55
Compare
|
@mergify requeue |
✅ This pull request will be embarked automaticallyThe head sha of this pull request, 6083c55, was never embarked in the merge queue. But don't worry, Mergify will embark it automatically for you. |
sanderegg
force-pushed
the
autoscaling/refactoring
branch
from
482daf4 to
d8a0d4b
Compare
|
|
This pull request has been removed from the queue for the following reason: The merge conditions cannot be satisfied due to failing checks:You may have to fix your CI before adding the pull request to the queue again. |
5 participants
What do these changes do?
This pull request refactors the autoscaling service to improve modularity and maintainability. Its main purpose is to simplify and de-confuse developers with regard to old naming conventions (namely usage of hot/warm buffer namings as per the current conventions).
There are no functionalities changed but the metrics names that the autoscaling-overview dashboard uses: related PR for devops are below.
Related issue/s
How to test
Dev-ops