Skip to content

Commit

Permalink
fix: issues with job failure and unique runs
Browse files Browse the repository at this point in the history
  • Loading branch information
SychO9 committed Dec 9, 2023
1 parent e48a1ac commit c9d2763
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 14 deletions.
2 changes: 1 addition & 1 deletion extensions/package-manager/extend.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
->default(Settings\LastUpdateRun::key(), json_encode(Settings\LastUpdateRun::default()))
->default('flarum-package-manager.queue_jobs', false)
->default('flarum-package-manager.minimum_stability', 'stable')
->default('flarum-package-manager.task_retention_days', 30),
->default('flarum-package-manager.task_retention_days', 7),

(new Extend\ServiceProvider)
->register(PackageManagerServiceProvider::class),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { Extension } from 'flarum/admin/AdminApplication';
import icon from 'flarum/common/helpers/icon';
import ItemList from 'flarum/common/utils/ItemList';
import extractText from 'flarum/common/utils/extractText';
import Link from 'flarum/common/components/Link';

import Label from './Label';
import TaskOutputModal from './TaskOutputModal';
Expand Down Expand Up @@ -73,15 +74,15 @@ export default class QueueSection extends Component<{}> {
const extension: Extension | null = app.data.extensions[task.package()?.replace(/(\/flarum-|\/flarum-ext-|\/)/g, '-')];

return extension ? (
<div className="PackageManager-queueTable-package">
<Link className="PackageManager-queueTable-package" href={app.route('extension', { id: extension.id })}>
<div className="PackageManager-queueTable-package-icon ExtensionIcon" style={extension.icon}>
{!!extension.icon && icon(extension.icon.name)}
</div>
<div className="PackageManager-queueTable-package-details">
<span className="PackageManager-queueTable-package-title">{extension.extra['flarum-extension'].title}</span>
<span className="PackageManager-queueTable-package-name">{task.package()}</span>
</div>
</div>
</Link>
) : (
task.package()
);
Expand Down Expand Up @@ -114,7 +115,7 @@ export default class QueueSection extends Component<{}> {
{
label: extractText(app.translator.trans('flarum-package-manager.admin.sections.queue.columns.elapsed_time')),
content: (task) =>
!task.startedAt() ? (
!task.startedAt() || !task.finishedAt() ? (
app.translator.trans('flarum-package-manager.admin.sections.queue.task_just_started')
) : (
<Tooltip text={`${dayjs(task.startedAt()).format('LL LTS')} ${dayjs(task.finishedAt()).format('LL LTS')}`}>
Expand Down
2 changes: 1 addition & 1 deletion extensions/package-manager/js/src/admin/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ app.initializers.add('flarum-package-manager', (app) => {
});
}}
>
Remove
{app.translator.trans('flarum-package-manager.admin.extensions.remove')}
</Button>
);
});
Expand Down
3 changes: 2 additions & 1 deletion extensions/package-manager/locale/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ flarum-package-manager:
install: Install a new extension
install_help: Fill in the extension package name to proceed. Visit {extiverse} to browse extensions.
proceed: Proceed
remove: Uninstall
successful_install: "{extension} was installed successfully, redirecting.."
successful_remove: Extension removed successfully.
successful_update: "{extension} was updated successfully, redirecting.."
Expand Down Expand Up @@ -59,7 +60,7 @@ flarum-package-manager:
columns:
details: Details
elapsed_time: Completed in
peak_memory_used: Maximum Memory Used
peak_memory_used: Peak Memory Usage
operation: Operation
package: Package
status: Status
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,10 @@ public function handle(CheckForUpdates $command)
$firstOutput = $this->runComposerCommand(false, $command);
$firstOutput = json_decode($this->cleanJson($firstOutput), true);

$installed = $firstOutput['installed'] ?? [];
$majorUpdates = false;

foreach ($firstOutput['installed'] as $package) {
foreach ($installed as $package) {
if (isset($package['latest-status']) && $package['latest-status'] === 'update-possible' && Util::isMajorUpdate($package['version'], $package['latest'])) {
$majorUpdates = true;
break;
Expand All @@ -74,7 +75,7 @@ public function handle(CheckForUpdates $command)
$secondOutput = ['installed' => []];
}

foreach ($firstOutput['installed'] as &$mainPackageUpdate) {
foreach ($installed as &$mainPackageUpdate) {
$mainPackageUpdate['latest-minor'] = $mainPackageUpdate['latest-major'] = null;

if ($mainPackageUpdate['latest-status'] === 'up-to-date') {
Expand All @@ -97,7 +98,7 @@ public function handle(CheckForUpdates $command)
}

return $this->lastUpdateCheck
->with('installed', $firstOutput['installed'])
->with('installed', $installed)
->save();
}

Expand Down
10 changes: 7 additions & 3 deletions extensions/package-manager/src/Job/ComposerCommandJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@
use Flarum\PackageManager\Command\AbstractActionCommand;
use Flarum\PackageManager\Composer\ComposerAdapter;
use Flarum\Queue\AbstractJob;
use Illuminate\Contracts\Queue\ShouldBeUnique;
use Illuminate\Queue\Middleware\WithoutOverlapping;
use Throwable;

class ComposerCommandJob extends AbstractJob
class ComposerCommandJob extends AbstractJob implements ShouldBeUnique
{
/**
* @var AbstractActionCommand
Expand Down Expand Up @@ -56,11 +57,14 @@ public function abort(Throwable $exception)
}

$this->command->task->end(false);
}

$this->fail($exception);
public function failed(Throwable $exception): void
{
$this->command->task->end(false);
}

public function middleware()
public function middleware(): array
{
return [
new WithoutOverlapping(),
Expand Down
2 changes: 2 additions & 0 deletions extensions/package-manager/src/Job/Dispatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ public function dispatch(AbstractActionCommand $command): DispatcherResponse
{
$queueJobs = ($this->runSyncOverride === false) || ($this->runSyncOverride !== true && $this->settings->get('flarum-package-manager.queue_jobs'));

// @todo: check and skip if there is already a pending or running task.

if ($queueJobs && (! $this->queue instanceof SyncQueue)) {
$task = Task::build($command->getOperationName(), $command->package ?? null);

Expand Down
12 changes: 10 additions & 2 deletions extensions/package-manager/src/Task/Task.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
* @property string $package
* @property string $output
* @property Carbon $created_at
* @property Carbon $started_at
* @property Carbon $finished_at
* @property Carbon|null $started_at
* @property Carbon|null $finished_at
* @property float $peak_memory_used
*/
class Task extends AbstractModel
Expand Down Expand Up @@ -84,6 +84,14 @@ public function start(): bool

public function end(bool $success): bool
{
if ($this->finished_at) {
return true;
}

if (! $this->started_at) {
$this->start();
}

$this->status = $success ? static::SUCCESS : static::FAILURE;
$this->finished_at = Carbon::now();
$this->peak_memory_used = round(memory_get_peak_usage() / 1024);
Expand Down

0 comments on commit c9d2763

Please sign in to comment.