Skip to content

Commit

Permalink
feat(@angular-devkit/core): remove Log messages from Job API
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hansl committed Feb 19, 2019
1 parent 0c549df commit 1b4a67f
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 56 deletions.
15 changes: 13 additions & 2 deletions packages/angular_devkit/architect/src/create-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -17,6 +17,7 @@ import {
BuilderProgressState,
Target,
TypedBuilderProgress,
targetStringFromTarget,
} from './api';
import { Builder, BuilderSymbol, BuilderVersionSymbol } from './internal';
import { scheduleByName, scheduleByTarget } from './schedule-by-name';
Expand All @@ -28,13 +29,16 @@ export function createBuilder<OptT extends json.JsonObject>(
const cjh = experimental.jobs.createJobHandler;
const handler = cjh<json.JsonObject, BuilderInput, BuilderOutput>((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) {
Expand Down Expand Up @@ -74,6 +78,13 @@ export function createBuilder<OptT extends json.JsonObject>(

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,
Expand Down
11 changes: 6 additions & 5 deletions packages/angular_devkit/architect/src/schedule-by-name.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,16 @@ export async function scheduleByName(
job.input.next(message);
}

const logChannelSub = job.getChannel<logging.LogEntry>('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();
}
Expand Down
8 changes: 1 addition & 7 deletions packages/angular_devkit/core/src/experimental/jobs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -162,8 +158,7 @@ export const add = jobs.createJobHandler<number[], number>(
```

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:

Expand Down Expand Up @@ -246,7 +241,6 @@ member of the `JobOutboundMessage<O>` 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
Expand Down
16 changes: 0 additions & 16 deletions packages/angular_devkit/core/src/experimental/jobs/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -135,7 +134,6 @@ export enum JobOutboundMessageKind {
Pong = 'p',

// Feedback messages.
Log = 'l',
Output = 'o',

// Channel specific messages.
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -274,7 +264,6 @@ export interface JobOutboundMessagePong extends JobOutboundMessageBase {
export type JobOutboundMessage<OutputT extends JsonValue> =
JobOutboundMessageOnReady
| JobOutboundMessageStart
| JobOutboundMessageLog
| JobOutboundMessageOutput<OutputT>
| JobOutboundMessageChannelCreate
| JobOutboundMessageChannelMessage
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ and output values (STDOUT) as well as diagnostic (STDERR). They can be plugged o
,______________________
JobInboundMessage<I> --> | handler(argument: A) | --> JobOutboundMessage<O>
`⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻ - JobOutboundMessageKind.Log
- JobOutboundMessageKind.Output
- ...
```
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -37,7 +37,6 @@ export interface SimpleJobHandlerContext<
I extends JsonValue,
O extends JsonValue,
> extends JobHandlerContext<A, I, O> {
logger: LoggerApi;
createChannel: (name: string) => Observer<JsonValue>;
input: Observable<I>;
}
Expand Down Expand Up @@ -100,23 +99,12 @@ export function createJobHandler<A extends JsonValue, I extends JsonValue, O ext
}
});

// Configure a logger to pass in as additional context.
const logger = new Logger('-internal-job-logger-');
const logSub = logger.subscribe(entry => {
subject.next({
kind: JobOutboundMessageKind.Log,
description,
entry,
});
});

// Execute the function with the additional context.
const channels = new Map<string, Subject<JsonValue>>();

const newContext: SimpleJobHandlerContext<A, I, O> = {
...context,
input: inputChannel.asObservable(),
logger,
createChannel(name: string) {
if (channels.has(name)) {
throw new ChannelAlreadyExistException(name);
Expand Down Expand Up @@ -164,7 +152,6 @@ export function createJobHandler<A extends JsonValue, I extends JsonValue, O ext
() => complete(),
);
subscription.add(inboundSub);
subscription.add(logSub);

return subscription;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<JsonValue>();
input.pipe(
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 1b4a67f

Please sign in to comment.