feat: add configurable stop grace period for application containers #7125
Conversation
|
Hi @andrasbacsai and @peaklabs-dev 👋 This is my first contribution here to Coolify :) We've been using Coolify for a couple years now and are rather happy with it. We are moving some services from AWS to a Hetzner-hosted Coolify instance. While going through production-readiness we noticed our new way of handling messages is part of the application deployment flow on Coolify, which means that when a new deployment comes in, the hardcoded 30 seconds of timeout is provided to Overall the rolling deployment process is great, but some of our messages have a timeout of 5 minutes as there can be quite a lot happening there, the 30s timeout opens up an issue if we initiate the action on an external service before stopping the container as we are then not able to wait the full time for the response, meaning the action will end up being performed twice after the message becomes visible on the queue again. This just isn't an acceptable outcome in our situation and there doesn't seem to be a great way to work around this in our architecture at the moment. It wasn't a problem on AWS as new deployments did not stop existing Lambdas triggered by SQS and they instead were allowed to complete with their normal timeouts while new lambdas picked up any new messages. @andrasbacsai - It looks like you were considering doing something like this before (source):
I have no idea if this is in alignment with how you would have added the feature. I'm totally open to feedback and will happily fix it up to make it mergeable as it's quite important for us to have this functionality. Let me know what you think :) |
Allow users to configure the Docker stop timeout (1-3600 seconds) for graceful container shutdown during rolling updates, manual stops, and restarts. Defaults to 30 seconds when not configured. - Add `stop_grace_period` field to `ApplicationSettings` (nullable integer) - Add UI control in Advanced settings with validation - Apply timeout to all container stop operations (deployments, manual stops, previews) - Add test coverage for model casting and null handling
Jackman3005
force-pushed
the
feat/configurable-stop-grace-period
branch
from
c68089f to
35d7f52
Compare
- It was hardcoded in numerous areas as well as in strings/placeholders shown to the user. In case it was ever changed it would be nice that all locations would appropriately be updated and not cause confusion.
|
Oh, I just realized I didn't consider adding any doc updates as part of this PR. The things of note are that this only applies to Applications, not Services and that |
|
Another small detail is that |
Hey, I have an PR for that! - #6807 |
|
@CodeRabbit review!!!!! |
|
@ShadowArcanist: I'll review the changes in this PR right away! ✅ Actions performedReview triggered.
|
|
Hi @Jackman3005! 👋 It appears to us that you are adding a new feature to Coolify. Coolify Docs Repository: https://github.com/coollabsio/coolify-docs |
WalkthroughThis pull request introduces a configurable stop grace period for application containers, replacing a hardcoded 30-second timeout. The feature includes a new database column, model casting, Livewire component with UI, and updates to container stop logic across multiple deployment and preview flows. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as Advanced View
participant Livewire as Advanced Component
participant Settings as ApplicationSetting
User->>UI: Input stop grace period (1-3600 sec)
User->>UI: Click Save
UI->>Livewire: saveStopGracePeriod()
Livewire->>Livewire: Normalize input (empty/null → null)
Livewire->>Livewire: Validate range (1-3600)
alt Validation passes
Livewire->>Settings: Persist to stop_grace_period
Settings->>Settings: Cast to integer
Livewire->>UI: Success notification
else Validation fails
Livewire->>UI: Error notification
end
sequenceDiagram
participant Container as Container Stop Flow
participant Setting as ApplicationSetting
participant Docker as Docker
Note over Container: Old: timeout = 30 sec (hardcoded)
Note over Container: New: timeout = configurable
Container->>Setting: Read stop_grace_period
alt Setting exists
Setting-->>Container: timeout = stop_grace_period
else Setting not set
Setting-->>Container: timeout = DEFAULT_STOP_GRACE_PERIOD_SECONDS (30)
end
Container->>Docker: docker stop --time={timeout} container
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/Unit/ApplicationSettingStaticCastTest.php (1)
107-128: Good test coverage, consider adding edge case tests.The tests properly verify the integer casting behavior and null handling. To ensure robustness, consider adding test cases for:
- Zero value:
$setting->stop_grace_period = 0(should cast to int 0)- Negative value:
$setting->stop_grace_period = -10(should cast to int -10)These edge cases would help document the expected behavior when invalid values are stored in the database.
Example tests to add:
it('casts stop_grace_period zero to integer', function () { $setting = new ApplicationSetting; $setting->stop_grace_period = 0; expect($setting->stop_grace_period)->toBe(0) ->and($setting->stop_grace_period)->toBeInt(); }); it('casts stop_grace_period negative value to integer', function () { $setting = new ApplicationSetting; $setting->stop_grace_period = -10; expect($setting->stop_grace_period)->toBe(-10) ->and($setting->stop_grace_period)->toBeInt(); });app/Livewire/Project/Application/Previews.php (1)
278-278: Add defensive validation check for edge case: prevent zero or negative timeout values.While validation exists in Advanced.php's update handler (lines 263-264), it only protects that save path. With no model-level validation or database constraints, a zero or negative value could bypass the check if set through direct database modification or a future code path. The
??operator at usage points only catches null, not invalid values.The suggested fix remains valid:
- $timeout = $this->application->settings->stop_grace_period ?? DEFAULT_STOP_GRACE_PERIOD_SECONDS; + $timeout = $this->application->settings->stop_grace_period > 0 + ? $this->application->settings->stop_grace_period + : DEFAULT_STOP_GRACE_PERIOD_SECONDS;Alternatively, add model-level validation in ApplicationSetting to enforce the constraint once, preventing the issue across all save paths.
resources/views/livewire/project/application/advanced.blade.php (1)
46-60: Ensure stop grace period value binds to Livewire state
Confirm that<x-forms.input id="stopGracePeriod" … />actually binds to the$stopGracePeriodproperty (e.g., viawire:model). Without a binding, the submitted value will remain empty and the new save handler will keep persistingnull. If the component does not auto-bind byid, add the explicitwire:model.defer="stopGracePeriod"(or equivalent).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/Actions/Application/StopApplication.php(1 hunks)app/Actions/Application/StopApplicationOneServer.php(1 hunks)app/Jobs/ApplicationDeploymentJob.php(1 hunks)app/Livewire/Project/Application/Advanced.php(3 hunks)app/Livewire/Project/Application/Previews.php(1 hunks)app/Models/ApplicationSetting.php(1 hunks)bootstrap/helpers/constants.php(1 hunks)database/migrations/2025_11_05_091558_add_stop_grace_period_to_application_settings.php(1 hunks)resources/views/livewire/project/application/advanced.blade.php(1 hunks)tests/Unit/ApplicationSettingStaticCastTest.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (26)
bootstrap/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/application-architecture.mdc)
Initialization/bootstrap code should reside under bootstrap/ (e.g., app bootstrapping)
Files:
bootstrap/helpers/constants.php
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)
**/*.php: Follow PSR-12 coding standards for all PHP code
Format PHP code with Laravel Pint configuration
Run static analysis with PHPStan to ensure type safety in PHP code
Document complex methods with PHPDoc blocks including parameters, return types, and thrown exceptions
**/*.php: Follow PSR-12 PHP coding standards across the codebase
Prefer eager loading and query optimization to prevent N+1 issues in database interactions
Use Laravel best practices for structure, services, and policies
**/*.php: Always use curly braces for control structures in PHP, even for single-line bodies
Use PHP 8 constructor property promotion in __construct() and avoid empty constructors
Always declare explicit return types for functions and methods; add appropriate parameter type hints
Prefer PHPDoc blocks over inline comments; only add inline code comments for very complex logic
When documenting arrays, add useful array shape types in PHPDoc where appropriate
Enum case names should be TitleCase (e.g., FavoritePerson, Monthly)
Files:
bootstrap/helpers/constants.phptests/Unit/ApplicationSettingStaticCastTest.phpdatabase/migrations/2025_11_05_091558_add_stop_grace_period_to_application_settings.phpapp/Livewire/Project/Application/Advanced.phpapp/Livewire/Project/Application/Previews.phpapp/Actions/Application/StopApplicationOneServer.phpapp/Jobs/ApplicationDeploymentJob.phpapp/Models/ApplicationSetting.phpresources/views/livewire/project/application/advanced.blade.phpapp/Actions/Application/StopApplication.php
tests/Unit/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/application-architecture.mdc)
Place unit tests under tests/Unit and keep them focused on individual classes/methods
tests/Unit/**/*.php: Unit tests (tests/Unit) must not use database connections or model factories
Unit tests must use mocking for models and external dependencies
Files:
tests/Unit/ApplicationSettingStaticCastTest.php
tests/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)
Write feature tests for API endpoints using Pest; mock external services and assert JSON responses and side effects
Add tests validating that components respect authorization (e.g., unauthorized users see disabled inputs; checkbox instantSave becomes false)
Include security tests covering SQL injection resistance, XSS validation, and enforcement of team isolation (403 on cross-team access).
Write tests using Pest (feature/unit) and include Laravel Dusk for browser scenarios as needed
Prefer Pest PHP as the primary testing framework for writing tests
Files:
tests/Unit/ApplicationSettingStaticCastTest.php
tests/**
📄 CodeRabbit inference engine (.cursor/rules/project-overview.mdc)
Keep automated tests (Pest/Dusk) under tests
Files:
tests/Unit/ApplicationSettingStaticCastTest.php
tests/{Feature,Unit}/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/laravel-boost.mdc)
tests/{Feature,Unit}/**/*.php: All tests must be written using Pest and live under tests/Feature or tests/Unit
Use specific response assertion helpers (e.g., assertForbidden, assertNotFound) instead of generic assertStatus
Use datasets in Pest to reduce duplication where suitable (e.g., validation tests)
Write or update unit/feature tests for every change and run the affected tests
Files:
tests/Unit/ApplicationSettingStaticCastTest.php
database/migrations/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)
Create descriptive migrations with proper foreign keys, cascading deletes, indexes, and timestamps
database/migrations/**/*.php: When modifying a column in a migration, re-specify all previously defined attributes to avoid dropping them
Use php artisan make: commands (e.g., make:migration) to create new Laravel files; pass --no-interaction and appropriate options
Files:
database/migrations/2025_11_05_091558_add_stop_grace_period_to_application_settings.php
database/migrations/**
📄 CodeRabbit inference engine (.cursor/rules/project-overview.mdc)
Keep database migrations under database/migrations
Files:
database/migrations/2025_11_05_091558_add_stop_grace_period_to_application_settings.php
database/migrations/*.php
📄 CodeRabbit inference engine (.cursor/rules/database-patterns.mdc)
database/migrations/*.php: Place all Laravel database migrations under database/migrations and follow standard Schema::create/Schema::table structure
Always provide both up() and down() methods in migrations to ensure rollback support
Use incremental, reversible migrations; separate structural changes from data migrations when appropriate
Create appropriate indexes: primary keys, foreign key indexes, composite indexes for common queries, and unique constraints for business rules
Files:
database/migrations/2025_11_05_091558_add_stop_grace_period_to_application_settings.php
app/Livewire/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/application-architecture.mdc)
Define Livewire components in app/Livewire and keep HTTP/UI concerns there
app/Livewire/**/*.php: Livewire components must authorize access in mount and before actions that mutate state
Expose only necessary public typed properties and register event listeners via the $listeners array in Livewire componentsPlace Livewire components under app/Livewire/ (e.g., Dashboard.php, ActivityMonitor.php, MonacoEditor.php)
Livewire 3 components should use the App\Livewire namespace and lifecycle hooks like mount() and updatedFoo()
Files:
app/Livewire/Project/Application/Advanced.phpapp/Livewire/Project/Application/Previews.php
app/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)
Use database transactions to group related write operations for consistency in services/controllers/jobs
Files:
app/Livewire/Project/Application/Advanced.phpapp/Livewire/Project/Application/Previews.phpapp/Actions/Application/StopApplicationOneServer.phpapp/Jobs/ApplicationDeploymentJob.phpapp/Models/ApplicationSetting.phpapp/Actions/Application/StopApplication.php
app/Livewire/**
📄 CodeRabbit inference engine (.cursor/rules/project-overview.mdc)
Implement frontend components as Livewire classes under app/Livewire
Files:
app/Livewire/Project/Application/Advanced.phpapp/Livewire/Project/Application/Previews.php
{app/Http/Controllers/**/*.php,app/Livewire/**/*.php}
📄 CodeRabbit inference engine (.cursor/rules/README.mdc)
Authorize sensitive actions with policies (e.g., $this->authorize('deploy', $model)) before invoking services
Files:
app/Livewire/Project/Application/Advanced.phpapp/Livewire/Project/Application/Previews.php
app/Actions/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/application-architecture.mdc)
Encapsulate business logic using the Action pattern; Action classes live in app/Actions and (via lorisleiva/laravel-actions) may expose handle()/asJob() methods
Files:
app/Actions/Application/StopApplicationOneServer.phpapp/Actions/Application/StopApplication.php
app/Actions/Application/*.php
📄 CodeRabbit inference engine (.cursor/rules/deployment-architecture.mdc)
Place core application deployment logic in app/Actions/Application
Files:
app/Actions/Application/StopApplicationOneServer.phpapp/Actions/Application/StopApplication.php
app/Actions/**
📄 CodeRabbit inference engine (.cursor/rules/project-overview.mdc)
Put business logic actions under app/Actions
Files:
app/Actions/Application/StopApplicationOneServer.phpapp/Actions/Application/StopApplication.php
app/Jobs/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/application-architecture.mdc)
Queueable background work should be implemented as Jobs under app/Jobs; Actions may expose asJob to dispatch
app/Jobs/**/*.php: Queue jobs should implement ShouldQueue and define retry strategy (tries, maxExceptions, backoff)
Queue jobs must handle success/failure state updates and broadcast corresponding events; implement failed(Throwable) for final failure handlingQueue jobs should update status, wrap external calls in try/catch, set failure status on exceptions, and rethrow
Files:
app/Jobs/ApplicationDeploymentJob.php
app/Jobs/*.php
📄 CodeRabbit inference engine (.cursor/rules/deployment-architecture.mdc)
Implement background deployment, monitoring, backup, and notification work as Laravel Job classes under app/Jobs
Files:
app/Jobs/ApplicationDeploymentJob.php
app/Jobs/**
📄 CodeRabbit inference engine (.cursor/rules/project-overview.mdc)
Implement background jobs under app/Jobs
Files:
app/Jobs/ApplicationDeploymentJob.php
app/Models/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/application-architecture.mdc)
Keep Eloquent domain models in app/Models; models represent core entities (e.g., Application, Server, Service, Team, Project)
app/Models/**/*.php: Define $fillable for mass assignment protection in Eloquent models
Use $casts for attribute casting (e.g., arrays, enums, datetimes) in Eloquent models
Define relationships using Eloquent relation return types (e.g., BelongsTo, HasMany)
app/Models/**/*.php: Define Eloquent relationships using proper relationship methods with return type hints
Prefer Eloquent ORM and relationships over raw queries or DB:: calls
Prevent N+1 queries by using eager loading when appropriate
When creating new models, also create useful factories and seeders
Files:
app/Models/ApplicationSetting.php
app/Models/**
📄 CodeRabbit inference engine (.cursor/rules/project-overview.mdc)
Place domain models under app/Models
Files:
app/Models/ApplicationSetting.php
app/Models/*.php
📄 CodeRabbit inference engine (.cursor/rules/database-patterns.mdc)
app/Models/*.php: When adding new database columns, update the model's $fillable array to allow mass assignment (e.g., for Model::create() and $model->update())
Use UUID primary keys via HasUuids for distributed systems
Enable soft deletes on models that require audit trails using SoftDeletes
Log model activity using the Spatie LogsActivity trait where auditing is required
Define explicit Eloquent relationships (belongsTo/hasMany/etc.) for navigable associations used by queries and eager loading
Files:
app/Models/ApplicationSetting.php
resources/views/**/*.blade.php
📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)
Use semantic Tailwind CSS classes and consistent spacing in Blade templates
resources/views/**/*.blade.php: In Blade views, prefer using x-forms components with canGate and :canResource instead of wrapping elements in @can/@else blocks
Remove legacy @can/@else blocks around individual form inputs/buttons and migrate to the single-line component pattern
Choose gates consistently in views: use update for configuration changes, deploy for operational actions, view for read-only access, and delete for destructive actions
Always pass the specific resource to :canResource (e.g., :canResource="$application", "$service", "$server"), and use team context for creation permissions (e.g., :canResource="auth()->user()->currentTeam" with canGate="createAnyResource")
Only set autoDisable="false" when also providing explicit :disabled logic on the component
Use single-line authorized components for forms (inputs, selects, checkboxes, buttons) to reduce duplication (e.g., <x-forms.input canGate="update" :canResource="$resource" ... />)
resources/views/**/*.blade.php: In Blade views, prefer x-forms.* components with canGate and :canResource for authorization (autoDisable defaults to true) instead of manual permission blocks.
Do not wrap x-forms.* components in @can/@else to toggle disabled state; use component-level canGate/:canResource instead.
For custom interactive components not using x-forms.* (e.g., Alpine.js widgets), wrap with @can('gate', $resource) and provide a disabled/readonly fallback in @else with opacity-50 and cursor-not-allowed styling.
resources/views/**/*.blade.php: Prefer named routes and the route() helper when generating links in views
Dispatch events using $this->dispatch() (not emit/dispatchBrowserEvent) in Livewire 3 templates/components
Do not manually include Alpine.js; Livewire 3 bundles Alpine with required plugins
Files:
resources/views/livewire/project/application/advanced.blade.php
resources/**
📄 CodeRabbit inference engine (.cursor/rules/project-overview.mdc)
Store frontend assets and views under resources
Files:
resources/views/livewire/project/application/advanced.blade.php
resources/views/livewire/**/*.blade.php
📄 CodeRabbit inference engine (.cursor/rules/laravel-boost.mdc)
Livewire components must have a single root element; use wire:key in loops and wire:loading/wire:dirty for UX
Files:
resources/views/livewire/project/application/advanced.blade.php
resources/**/*.{blade.php,vue}
📄 CodeRabbit inference engine (.cursor/rules/laravel-boost.mdc)
Use Tailwind gap utilities for spacing lists instead of margins when listing items
Files:
resources/views/livewire/project/application/advanced.blade.php
🧠 Learnings (5)
📚 Learning: 2025-08-27T15:01:58.710Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/development-workflow.mdc:0-0
Timestamp: 2025-08-27T15:01:58.710Z
Learning: Applies to app/Models/**/*.php : Use $casts for attribute casting (e.g., arrays, enums, datetimes) in Eloquent models
Applied to files:
tests/Unit/ApplicationSettingStaticCastTest.phpapp/Models/ApplicationSetting.php
📚 Learning: 2025-10-15T09:16:25.875Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/laravel-boost.mdc:0-0
Timestamp: 2025-10-15T09:16:25.875Z
Learning: Applies to database/migrations/**/*.php : When modifying a column in a migration, re-specify all previously defined attributes to avoid dropping them
Applied to files:
database/migrations/2025_11_05_091558_add_stop_grace_period_to_application_settings.php
📚 Learning: 2025-08-27T15:01:58.710Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/development-workflow.mdc:0-0
Timestamp: 2025-08-27T15:01:58.710Z
Learning: Applies to database/migrations/**/*.php : Create descriptive migrations with proper foreign keys, cascading deletes, indexes, and timestamps
Applied to files:
database/migrations/2025_11_05_091558_add_stop_grace_period_to_application_settings.php
📚 Learning: 2025-10-15T09:14:50.127Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/database-patterns.mdc:0-0
Timestamp: 2025-10-15T09:14:50.127Z
Learning: Applies to database/migrations/*.php : Always provide both up() and down() methods in migrations to ensure rollback support
Applied to files:
database/migrations/2025_11_05_091558_add_stop_grace_period_to_application_settings.php
📚 Learning: 2025-10-15T09:14:50.127Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/database-patterns.mdc:0-0
Timestamp: 2025-10-15T09:14:50.127Z
Learning: Applies to database/migrations/*.php : Use incremental, reversible migrations; separate structural changes from data migrations when appropriate
Applied to files:
database/migrations/2025_11_05_091558_add_stop_grace_period_to_application_settings.php
🔇 Additional comments (6)
app/Models/ApplicationSetting.php (1)
26-26: LGTM! Proper integer casting applied.The stop_grace_period field is correctly cast as an integer, following the established pattern in the model and adhering to coding guidelines.
database/migrations/2025_11_05_091558_add_stop_grace_period_to_application_settings.php (1)
14-19: LGTM! Well-structured migration with clear documentation.The migration properly adds the nullable integer column with a descriptive comment that clearly documents the valid range and default behavior. Both up() and down() methods are present for rollback support.
bootstrap/helpers/constants.php (1)
19-19: LGTM! Well-named constant with appropriate default value.The constant is properly named, follows existing conventions, and the 30-second default maintains backward compatibility with the previous hardcoded timeout.
app/Jobs/ApplicationDeploymentJob.php (1)
2991-2995: Good dev environment optimization, but verify edge case handling.The special case for
isDev()with a 1-second timeout is a good optimization for development environments. However, the same edge case concern applies here: ifstop_grace_periodis set to 0 or a negative number in production, it will bypass the fallback toDEFAULT_STOP_GRACE_PERIOD_SECONDS.Consider adding validation:
if (isDev()) { $timeout = 1; } else { - $timeout = $this->application->settings->stop_grace_period ?? DEFAULT_STOP_GRACE_PERIOD_SECONDS; + $timeout = ($this->application->settings->stop_grace_period > 0) + ? $this->application->settings->stop_grace_period + : DEFAULT_STOP_GRACE_PERIOD_SECONDS; }app/Actions/Application/StopApplicationOneServer.php (1)
23-23: Consistent implementation, but same edge case concern.The implementation follows the same pattern as other files in this PR. However, the same validation concern applies: the
??operator won't catch 0 or negative values. See the comment onapp/Livewire/Project/Application/Previews.phpfor details and suggested fix.app/Actions/Application/StopApplication.php (1)
39-39: Consistent pattern across all stop actions.The timeout derivation is consistent with all other files in this PR. The same validation recommendation applies here to handle edge cases where
stop_grace_periodmight be 0 or negative.
| // Load stop_grace_period separately since it has its own save handler | ||
| // Convert null to empty string to prevent dirty detection issues | ||
| $this->stopGracePeriod = $this->application->settings->stop_grace_period ?? ''; | ||
| } |
There was a problem hiding this comment.
Fix type mismatch when loading stop grace period
stop_grace_period is cast to integer on the model, but here you assign it directly to public ?string $stopGracePeriod. When the stored value is, say, 60, Livewire will throw TypeError: Cannot assign int to property App\Livewire\Project\Application\Advanced::$stopGracePeriod of type ?string. Cast the value to string (or change the property type) before assignment, e.g.:
- $this->stopGracePeriod = $this->application->settings->stop_grace_period ?? '';
+ $value = $this->application->settings->stop_grace_period;
+ $this->stopGracePeriod = $value === null ? '' : (string) $value;🤖 Prompt for AI Agents
In app/Livewire/Project/Application/Advanced.php around lines 142 to 145,
assigning the integer model field stop_grace_period directly to the property
typed as ?string causes a TypeError in Livewire; convert the loaded value to a
string (e.g. (string)$this->application->settings->stop_grace_period) before
assigning, or alternatively change the property type to allow int, so that the
assignment matches the declared property type.
There was a problem hiding this comment.
I'll look into this. I think it was working without a cast as I could refresh the page with a number configured and it worked.
The reason I use a string here is the user cannot set the current value to null after they have entered any content. So to fix problems with the change indicator incorrectly identifying an empty string as a change from the previous value of null I found it easiest to convert to string and back again after. Happy to adjust if someone has a suggestion.
There was a problem hiding this comment.
@Jackman3005, you're right to be concerned about dirty detection, and your approach of converting null → '' is sound for that use case. However, the explicit cast is still necessary for type safety.
What's likely happening is that PHP or Livewire is performing implicit type coercion in your current setup, which can work in many cases but isn't guaranteed and can break with strict type checking enabled. Since stop_grace_period is cast to integer on the model, when the value is, say, 60, you're assigning int(60) to a ?string property, which should trigger a TypeError in strict PHP 8+.
The bot's suggestion actually preserves your dirty detection fix while adding the necessary type safety:
$value = $this->application->settings->stop_grace_period;
$this->stopGracePeriod = $value === null ? '' : (string) $value;This still converts null to '' (preventing dirty detection issues), but also explicitly casts non-null integers to strings (e.g., 60 → '60'), ensuring the assignment matches your ?string property type. Your saveStopGracePeriod() method already handles the reverse conversion ((int) $this->stopGracePeriod), so the round-trip behavior remains intact.
I'd recommend accepting the bot's suggestion—it maintains your intended behavior while being more robust.
|
one thing to note, docker already has options for stop_grace_period as well as stop_signal so a different solution would be to instead just add the option to the generated compose in app deployment (or use the user defined values in their own compose) and then remove the timeout altogether from the command line, since docker stop would by default use the value for that container. since the above pr would use the same setting for all services in a compose based application, but they might need different values, this might not be such a big issue though. |
|
@djsisson we're using Dockerfile build pack not docker compose. How would you imagine it working for that? I did look into respecting the docker compose |
|
@djsisson ahh maybe I understand what you're saying a bit. By removing Pardon me if I misunderstood I don't have the code in front of me. If so, I guess I'd need to check the different application types to make sure each get a chance to set the grace period either through the UI or via the user-supplied |
|
the compose is generated here: coolify/app/Jobs/ApplicationDeploymentJob.php Line 2327 in 7bbfa09 also i assume you would surround the view with it might be worth adding stop_signal as well |
3 participants
Description
This PR adds a configurable stop grace period for application containers, allowing users to specify how long Docker should wait for graceful shutdown before forcing containers to stop.
Previously, the stop timeout was hardcoded to 30 seconds, which was insufficient for applications that need more time for graceful shutdown (e.g., draining connections, finishing requests, cleanup operations). Users can now configure a custom timeout between 1 and 3600 seconds (1 hour) via the UI in Advanced settings.
The setting applies to:
When not configured, the system defaults to 30 seconds, maintaining backwards compatibility.
Changes
stop_grace_periodnullable integer field toapplication_settingstable (1-3600 range)stop_grace_periodproperty toApplicationSettingmodel with integer castgraceful_shutdown_container()inApplicationDeploymentJobto use configured timeoutStopApplicationaction to use configured timeoutStopApplicationOneServeraction to use configured timeoutPreviewscomponentstopContainers()to use configured timeoutApplicationSettingStaticCastTestfor model casting and null handlingDEFAULT_STOP_GRACE_PERIOD_SECONDS = 30constant tobootstrap/helpers/constants.php. Maybe there is a more appropriate location for this? Please let me know if so. All locations that hardcoded 30s as the default now use this constant as well as the 2 new locations in the UI where it is shown (tooltip and placeholder).2025_11_05_091558_add_stop_grace_period_to_application_settings.phpIssues
stop_grace_period#5620 (docker-composestop_grace_perioddiscussion). Doesn't resolve exactly as usingstop_grace_periodfrom the docker-compose.yml had two problems.Screenshots
Added to advanced options under new Operations section header
Empty is treated as
nullwhich falls back to default, so 30 is shown as placeholder since it is the default.Informative tooltip
Confirmed that value set and saved is used when calling
docker stopin a rolling update.Summary by CodeRabbit
New Features
Tests