From 1b4a67f63f17f47d8a8fff4079d103264782ce15 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Thu, 14 Feb 2019 13:29:39 -0800 Subject: [PATCH] feat(@angular-devkit/core): remove Log messages from Job API If a system wants to have logging it should multiplex it itself on a channel. Also changed the previous Architect commits to remove usage of Logs and move to a "log" channel. --- .../architect/src/create-builder.ts | 15 +++++++++++++-- .../architect/src/schedule-by-name.ts | 11 ++++++----- .../core/src/experimental/jobs/README.md | 8 +------- .../core/src/experimental/jobs/api.ts | 16 ---------------- .../core/src/experimental/jobs/architecture.md | 4 +--- .../src/experimental/jobs/create-job-handler.ts | 15 +-------------- .../src/experimental/jobs/simple-scheduler.ts | 10 +--------- 7 files changed, 23 insertions(+), 56 deletions(-) diff --git a/packages/angular_devkit/architect/src/create-builder.ts b/packages/angular_devkit/architect/src/create-builder.ts index 56053ad81a27..9ff2ff6824b8 100644 --- a/packages/angular_devkit/architect/src/create-builder.ts +++ b/packages/angular_devkit/architect/src/create-builder.ts @@ -5,7 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import { experimental, isPromise, json } from '@angular-devkit/core'; +import { experimental, isPromise, json, logging } from '@angular-devkit/core'; import { Observable, Subscription, from, isObservable, of } from 'rxjs'; import { tap } from 'rxjs/operators'; import { @@ -17,6 +17,7 @@ import { BuilderProgressState, Target, TypedBuilderProgress, + targetStringFromTarget, } from './api'; import { Builder, BuilderSymbol, BuilderVersionSymbol } from './internal'; import { scheduleByName, scheduleByTarget } from './schedule-by-name'; @@ -28,13 +29,16 @@ export function createBuilder( const cjh = experimental.jobs.createJobHandler; const handler = cjh((options, context) => { const scheduler = context.scheduler; - const logger = context.logger; const progressChannel = context.createChannel('progress'); + const logChannel = context.createChannel('log'); let currentState: BuilderProgressState = BuilderProgressState.Stopped; let current = 0; let status = ''; let total = 1; + function log(entry: logging.LogEntry) { + logChannel.next(entry); + } function progress(progress: TypedBuilderProgress, context: BuilderContext) { currentState = progress.state; if (progress.state === BuilderProgressState.Running) { @@ -74,6 +78,13 @@ export function createBuilder( function onInput(i: BuilderInput) { const builder = i.info as BuilderInfo; + const loggerName = i.target + ? targetStringFromTarget(i.target as Target) + : builder.builderName; + const logger = new logging.Logger(loggerName); + + subscriptions.push(logger.subscribe(entry => log(entry))); + const context: BuilderContext = { builder, workspaceRoot: i.workspaceRoot, diff --git a/packages/angular_devkit/architect/src/schedule-by-name.ts b/packages/angular_devkit/architect/src/schedule-by-name.ts index d3bc12b07040..0022c9b53207 100644 --- a/packages/angular_devkit/architect/src/schedule-by-name.ts +++ b/packages/angular_devkit/architect/src/schedule-by-name.ts @@ -65,15 +65,16 @@ export async function scheduleByName( job.input.next(message); } + const logChannelSub = job.getChannel('log').subscribe(entry => { + logger.next(entry); + }); + const s = job.outboundBus.subscribe( - message => { - if (message.kind == experimental.jobs.JobOutboundMessageKind.Log) { - logger.log(message.entry.level, message.entry.message, message.entry); - } - }, + undefined, undefined, () => { s.unsubscribe(); + logChannelSub.unsubscribe(); if (stateSubscription) { stateSubscription.unsubscribe(); } diff --git a/packages/angular_devkit/core/src/experimental/jobs/README.md b/packages/angular_devkit/core/src/experimental/jobs/README.md index 3989c30d0d38..e1640a5382b9 100644 --- a/packages/angular_devkit/core/src/experimental/jobs/README.md +++ b/packages/angular_devkit/core/src/experimental/jobs/README.md @@ -19,10 +19,6 @@ The I/O model is like that of an executable, where the argument corresponds to a command line, the input channel to STDIN, the output channel to STDOUT, and the channels would be additional output streams. -In addition, a `Job` has a logging channel that can be used to log messages to the user. The -code that schedules the job must listen for or forward these messages. You can think of those -messages as STDERR. - ## LifeCycle A `Job` goes through multiple LifeCycle messages before its completion; 1. `JobState.Queued`. The job was queued and is waiting. This is the default state from the @@ -162,8 +158,7 @@ export const add = jobs.createJobHandler( ``` You can also return a Promise or an Observable, as jobs are asynchronous. This helper will set -start and end messages appropriately, and will pass in a logger. It will also manage channels -automatically (see below). +start and end messages appropriately. It will also manage channels automatically (see below). A more complex job can be declared like this: @@ -246,7 +241,6 @@ member of the `JobOutboundMessage` interface dictates what kind of message it dependent jobs. `createJobHandler()` automatically send this message. 1. `JobOutboundMessageKind.Pong`. The job should answer a `JobInboundMessageKind.Ping` message with this. Automatically done by `createJobHandler()`. -1. `JobOutboundMessageKind.Log`. A logging message (side effect that should be shown to the user). 1. `JobOutboundMessageKind.Output`. An `Output` has been generated by the job. 1. `JobOutboundMessageKind.ChannelMessage`, `JobOutboundMessageKind.ChannelError` and `JobOutboundMessageKind.ChannelComplete` are used for output channels. These correspond to the diff --git a/packages/angular_devkit/core/src/experimental/jobs/api.ts b/packages/angular_devkit/core/src/experimental/jobs/api.ts index 227c7e2eb51d..5d109bf0904b 100644 --- a/packages/angular_devkit/core/src/experimental/jobs/api.ts +++ b/packages/angular_devkit/core/src/experimental/jobs/api.ts @@ -7,7 +7,6 @@ */ import { Observable, Observer } from 'rxjs'; import { JsonObject, JsonValue, schema } from '../../json/index'; -import { LogEntry, LoggerApi } from '../../logger/index'; import { DeepReadonly } from '../../utils/index'; /** @@ -135,7 +134,6 @@ export enum JobOutboundMessageKind { Pong = 'p', // Feedback messages. - Log = 'l', Output = 'o', // Channel specific messages. @@ -172,14 +170,6 @@ export interface JobOutboundMessageStart extends JobOutboundMessageBase { readonly kind: JobOutboundMessageKind.Start; } -/** - * A logging message, supporting the logging.LogEntry. - */ -export interface JobOutboundMessageLog extends JobOutboundMessageBase { - readonly kind: JobOutboundMessageKind.Log; - readonly entry: LogEntry; -} - /** * An output value is available. */ @@ -274,7 +264,6 @@ export interface JobOutboundMessagePong extends JobOutboundMessageBase { export type JobOutboundMessage = JobOutboundMessageOnReady | JobOutboundMessageStart - | JobOutboundMessageLog | JobOutboundMessageOutput | JobOutboundMessageChannelCreate | JobOutboundMessageChannelMessage @@ -381,11 +370,6 @@ export interface Job< * Options for scheduling jobs. */ export interface ScheduleJobOptions { - /** - * Where should logging be passed in. By default logging will be dropped. - */ - logger?: LoggerApi; - /** * Jobs that need to finish before scheduling this job. These dependencies will be passed * to the job itself in its context. diff --git a/packages/angular_devkit/core/src/experimental/jobs/architecture.md b/packages/angular_devkit/core/src/experimental/jobs/architecture.md index 216339f167dd..ded5a0bd5f83 100644 --- a/packages/angular_devkit/core/src/experimental/jobs/architecture.md +++ b/packages/angular_devkit/core/src/experimental/jobs/architecture.md @@ -101,7 +101,6 @@ and output values (STDOUT) as well as diagnostic (STDERR). They can be plugged o ,______________________ JobInboundMessage --> | handler(argument: A) | --> JobOutboundMessage - `⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻ - JobOutboundMessageKind.Log - JobOutboundMessageKind.Output - ... ``` @@ -129,7 +128,6 @@ and output values (STDOUT) as well as diagnostic (STDERR). They can be plugged o unblock dependent jobs. `createJobHandler()` automatically send this message. 1. `JobOutboundMessageKind.Pong`. The job should answer a `JobInboundMessageKind.Ping` message with this. Automatically done by `createJobHandler()`. -1. `JobOutboundMessageKind.Log`. A logging message (side effect that should be shown to the user). 1. `JobOutboundMessageKind.Output`. An `Output` has been generated by the job. 1. `JobOutboundMessageKind.ChannelMessage`, `JobOutboundMessageKind.ChannelError` and `JobOutboundMessageKind.ChannelComplete` are used for output channels. These correspond to @@ -251,7 +249,7 @@ example would be an operator that takes a module path and run the job from that process. Or even a separate server, using HTTP calls. Another limitation is that the boilerplate is complex. Manually managing start/end life cycle, and -other messages such as logging, etc. is tedious and requires a lot of code. A good way to keep +other messages such as ping/pong, etc. is tedious and requires a lot of code. A good way to keep this limitation under control is to provide helpers to create `JobHandler`s which manage those messages for the developer. A simple handler could be to get a `Promise` and return the output of that promise automatically. diff --git a/packages/angular_devkit/core/src/experimental/jobs/create-job-handler.ts b/packages/angular_devkit/core/src/experimental/jobs/create-job-handler.ts index 40b63c94bd05..08f381dc081f 100644 --- a/packages/angular_devkit/core/src/experimental/jobs/create-job-handler.ts +++ b/packages/angular_devkit/core/src/experimental/jobs/create-job-handler.ts @@ -10,7 +10,7 @@ import { Observable, Observer, Subject, Subscription, from, isObservable, of } f import { switchMap, tap } from 'rxjs/operators'; import { BaseException } from '../../exception/index'; import { JsonValue } from '../../json/index'; -import { Logger, LoggerApi } from '../../logger/index'; +import { LoggerApi } from '../../logger'; import { isPromise } from '../../utils/index'; import { JobDescription, @@ -37,7 +37,6 @@ export interface SimpleJobHandlerContext< I extends JsonValue, O extends JsonValue, > extends JobHandlerContext { - logger: LoggerApi; createChannel: (name: string) => Observer; input: Observable; } @@ -100,23 +99,12 @@ export function createJobHandler { - subject.next({ - kind: JobOutboundMessageKind.Log, - description, - entry, - }); - }); - // Execute the function with the additional context. const channels = new Map>(); const newContext: SimpleJobHandlerContext = { ...context, input: inputChannel.asObservable(), - logger, createChannel(name: string) { if (channels.has(name)) { throw new ChannelAlreadyExistException(name); @@ -164,7 +152,6 @@ export function createJobHandler complete(), ); subscription.add(inboundSub); - subscription.add(logSub); return subscription; }); diff --git a/packages/angular_devkit/core/src/experimental/jobs/simple-scheduler.ts b/packages/angular_devkit/core/src/experimental/jobs/simple-scheduler.ts index 76ade7049bb1..9c1c4fb7d88a 100644 --- a/packages/angular_devkit/core/src/experimental/jobs/simple-scheduler.ts +++ b/packages/angular_devkit/core/src/experimental/jobs/simple-scheduler.ts @@ -22,13 +22,12 @@ import { filter, first, ignoreElements, - map, share, + map, shareReplay, switchMap, tap, } from 'rxjs/operators'; import { JsonValue, schema } from '../../json'; -import { NullLogger } from '../../logger'; import { Job, JobDescription, @@ -305,8 +304,6 @@ export class SimpleScheduler< let state = JobState.Queued; let pingId = 0; - const logger = options.logger ? options.logger : new NullLogger(); - // Create the input channel by having a filter. const input = new Subject(); input.pipe( @@ -348,11 +345,6 @@ export class SimpleScheduler< state = this._updateState(message, state); switch (message.kind) { - case JobOutboundMessageKind.Log: - const entry = message.entry; - logger.log(entry.level, entry.message, entry); - break; - case JobOutboundMessageKind.ChannelCreate: { const maybeSubject = channelsSubject.get(message.name); // If it doesn't exist or it's closed on the other end.