Skip to content

Commit 1aa1b47

Browse files
hanslKeen Yee Liau
authored and
Keen Yee Liau
committed
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.
1 parent 0f0f289 commit 1aa1b47

File tree

7 files changed

+23
-56
lines changed

7 files changed

+23
-56
lines changed

Diff for: packages/angular_devkit/architect/src/create-builder.ts

+13-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* Use of this source code is governed by an MIT-style license that can be
66
* found in the LICENSE file at https://angular.io/license
77
*/
8-
import { experimental, isPromise, json } from '@angular-devkit/core';
8+
import { experimental, isPromise, json, logging } from '@angular-devkit/core';
99
import { Observable, Subscription, from, isObservable, of } from 'rxjs';
1010
import { tap } from 'rxjs/operators';
1111
import {
@@ -17,6 +17,7 @@ import {
1717
BuilderProgressState,
1818
Target,
1919
TypedBuilderProgress,
20+
targetStringFromTarget,
2021
} from './api';
2122
import { Builder, BuilderSymbol, BuilderVersionSymbol } from './internal';
2223
import { scheduleByName, scheduleByTarget } from './schedule-by-name';
@@ -28,13 +29,16 @@ export function createBuilder<OptT extends json.JsonObject>(
2829
const cjh = experimental.jobs.createJobHandler;
2930
const handler = cjh<json.JsonObject, BuilderInput, BuilderOutput>((options, context) => {
3031
const scheduler = context.scheduler;
31-
const logger = context.logger;
3232
const progressChannel = context.createChannel('progress');
33+
const logChannel = context.createChannel('log');
3334
let currentState: BuilderProgressState = BuilderProgressState.Stopped;
3435
let current = 0;
3536
let status = '';
3637
let total = 1;
3738

39+
function log(entry: logging.LogEntry) {
40+
logChannel.next(entry);
41+
}
3842
function progress(progress: TypedBuilderProgress, context: BuilderContext) {
3943
currentState = progress.state;
4044
if (progress.state === BuilderProgressState.Running) {
@@ -74,6 +78,13 @@ export function createBuilder<OptT extends json.JsonObject>(
7478

7579
function onInput(i: BuilderInput) {
7680
const builder = i.info as BuilderInfo;
81+
const loggerName = i.target
82+
? targetStringFromTarget(i.target as Target)
83+
: builder.builderName;
84+
const logger = new logging.Logger(loggerName);
85+
86+
subscriptions.push(logger.subscribe(entry => log(entry)));
87+
7788
const context: BuilderContext = {
7889
builder,
7990
workspaceRoot: i.workspaceRoot,

Diff for: packages/angular_devkit/architect/src/schedule-by-name.ts

+6-5
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,16 @@ export async function scheduleByName(
6565
job.input.next(message);
6666
}
6767

68+
const logChannelSub = job.getChannel<logging.LogEntry>('log').subscribe(entry => {
69+
logger.next(entry);
70+
});
71+
6872
const s = job.outboundBus.subscribe(
69-
message => {
70-
if (message.kind == experimental.jobs.JobOutboundMessageKind.Log) {
71-
logger.log(message.entry.level, message.entry.message, message.entry);
72-
}
73-
},
73+
undefined,
7474
undefined,
7575
() => {
7676
s.unsubscribe();
77+
logChannelSub.unsubscribe();
7778
if (stateSubscription) {
7879
stateSubscription.unsubscribe();
7980
}

Diff for: packages/angular_devkit/core/src/experimental/jobs/README.md

+1-7
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,6 @@ The I/O model is like that of an executable, where the argument corresponds to a
1919
command line, the input channel to STDIN, the output channel to STDOUT, and the channels
2020
would be additional output streams.
2121

22-
In addition, a `Job` has a logging channel that can be used to log messages to the user. The
23-
code that schedules the job must listen for or forward these messages. You can think of those
24-
messages as STDERR.
25-
2622
## LifeCycle
2723
A `Job` goes through multiple LifeCycle messages before its completion;
2824
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<number[], number>(
162158
```
163159

164160
You can also return a Promise or an Observable, as jobs are asynchronous. This helper will set
165-
start and end messages appropriately, and will pass in a logger. It will also manage channels
166-
automatically (see below).
161+
start and end messages appropriately. It will also manage channels automatically (see below).
167162

168163
A more complex job can be declared like this:
169164

@@ -246,7 +241,6 @@ member of the `JobOutboundMessage<O>` interface dictates what kind of message it
246241
dependent jobs. `createJobHandler()` automatically send this message.
247242
1. `JobOutboundMessageKind.Pong`. The job should answer a `JobInboundMessageKind.Ping` message with
248243
this. Automatically done by `createJobHandler()`.
249-
1. `JobOutboundMessageKind.Log`. A logging message (side effect that should be shown to the user).
250244
1. `JobOutboundMessageKind.Output`. An `Output` has been generated by the job.
251245
1. `JobOutboundMessageKind.ChannelMessage`, `JobOutboundMessageKind.ChannelError` and
252246
`JobOutboundMessageKind.ChannelComplete` are used for output channels. These correspond to the

Diff for: packages/angular_devkit/core/src/experimental/jobs/api.ts

-16
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
*/
88
import { Observable, Observer } from 'rxjs';
99
import { JsonObject, JsonValue, schema } from '../../json/index';
10-
import { LogEntry, LoggerApi } from '../../logger/index';
1110
import { DeepReadonly } from '../../utils/index';
1211

1312
/**
@@ -135,7 +134,6 @@ export enum JobOutboundMessageKind {
135134
Pong = 'p',
136135

137136
// Feedback messages.
138-
Log = 'l',
139137
Output = 'o',
140138

141139
// Channel specific messages.
@@ -172,14 +170,6 @@ export interface JobOutboundMessageStart extends JobOutboundMessageBase {
172170
readonly kind: JobOutboundMessageKind.Start;
173171
}
174172

175-
/**
176-
* A logging message, supporting the logging.LogEntry.
177-
*/
178-
export interface JobOutboundMessageLog extends JobOutboundMessageBase {
179-
readonly kind: JobOutboundMessageKind.Log;
180-
readonly entry: LogEntry;
181-
}
182-
183173
/**
184174
* An output value is available.
185175
*/
@@ -274,7 +264,6 @@ export interface JobOutboundMessagePong extends JobOutboundMessageBase {
274264
export type JobOutboundMessage<OutputT extends JsonValue> =
275265
JobOutboundMessageOnReady
276266
| JobOutboundMessageStart
277-
| JobOutboundMessageLog
278267
| JobOutboundMessageOutput<OutputT>
279268
| JobOutboundMessageChannelCreate
280269
| JobOutboundMessageChannelMessage
@@ -381,11 +370,6 @@ export interface Job<
381370
* Options for scheduling jobs.
382371
*/
383372
export interface ScheduleJobOptions {
384-
/**
385-
* Where should logging be passed in. By default logging will be dropped.
386-
*/
387-
logger?: LoggerApi;
388-
389373
/**
390374
* Jobs that need to finish before scheduling this job. These dependencies will be passed
391375
* to the job itself in its context.

Diff for: packages/angular_devkit/core/src/experimental/jobs/architecture.md

+1-3
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@ and output values (STDOUT) as well as diagnostic (STDERR). They can be plugged o
101101
102102
,______________________
103103
JobInboundMessage<I> --> | handler(argument: A) | --> JobOutboundMessage<O>
104-
`⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻ - JobOutboundMessageKind.Log
105104
- JobOutboundMessageKind.Output
106105
- ...
107106
```
@@ -129,7 +128,6 @@ and output values (STDOUT) as well as diagnostic (STDERR). They can be plugged o
129128
unblock dependent jobs. `createJobHandler()` automatically send this message.
130129
1. `JobOutboundMessageKind.Pong`. The job should answer a `JobInboundMessageKind.Ping` message with
131130
this. Automatically done by `createJobHandler()`.
132-
1. `JobOutboundMessageKind.Log`. A logging message (side effect that should be shown to the user).
133131
1. `JobOutboundMessageKind.Output`. An `Output` has been generated by the job.
134132
1. `JobOutboundMessageKind.ChannelMessage`, `JobOutboundMessageKind.ChannelError` and
135133
`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
251249
process. Or even a separate server, using HTTP calls.
252250

253251
Another limitation is that the boilerplate is complex. Manually managing start/end life cycle, and
254-
other messages such as logging, etc. is tedious and requires a lot of code. A good way to keep
252+
other messages such as ping/pong, etc. is tedious and requires a lot of code. A good way to keep
255253
this limitation under control is to provide helpers to create `JobHandler`s which manage those
256254
messages for the developer. A simple handler could be to get a `Promise` and return the output of
257255
that promise automatically.

Diff for: packages/angular_devkit/core/src/experimental/jobs/create-job-handler.ts

+1-14
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { Observable, Observer, Subject, Subscription, from, isObservable, of } f
1010
import { switchMap, tap } from 'rxjs/operators';
1111
import { BaseException } from '../../exception/index';
1212
import { JsonValue } from '../../json/index';
13-
import { Logger, LoggerApi } from '../../logger/index';
13+
import { LoggerApi } from '../../logger';
1414
import { isPromise } from '../../utils/index';
1515
import {
1616
JobDescription,
@@ -37,7 +37,6 @@ export interface SimpleJobHandlerContext<
3737
I extends JsonValue,
3838
O extends JsonValue,
3939
> extends JobHandlerContext<A, I, O> {
40-
logger: LoggerApi;
4140
createChannel: (name: string) => Observer<JsonValue>;
4241
input: Observable<I>;
4342
}
@@ -100,23 +99,12 @@ export function createJobHandler<A extends JsonValue, I extends JsonValue, O ext
10099
}
101100
});
102101

103-
// Configure a logger to pass in as additional context.
104-
const logger = new Logger('-internal-job-logger-');
105-
const logSub = logger.subscribe(entry => {
106-
subject.next({
107-
kind: JobOutboundMessageKind.Log,
108-
description,
109-
entry,
110-
});
111-
});
112-
113102
// Execute the function with the additional context.
114103
const channels = new Map<string, Subject<JsonValue>>();
115104

116105
const newContext: SimpleJobHandlerContext<A, I, O> = {
117106
...context,
118107
input: inputChannel.asObservable(),
119-
logger,
120108
createChannel(name: string) {
121109
if (channels.has(name)) {
122110
throw new ChannelAlreadyExistException(name);
@@ -164,7 +152,6 @@ export function createJobHandler<A extends JsonValue, I extends JsonValue, O ext
164152
() => complete(),
165153
);
166154
subscription.add(inboundSub);
167-
subscription.add(logSub);
168155

169156
return subscription;
170157
});

Diff for: packages/angular_devkit/core/src/experimental/jobs/simple-scheduler.ts

+1-9
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,12 @@ import {
2222
filter,
2323
first,
2424
ignoreElements,
25-
map, share,
25+
map,
2626
shareReplay,
2727
switchMap,
2828
tap,
2929
} from 'rxjs/operators';
3030
import { JsonValue, schema } from '../../json';
31-
import { NullLogger } from '../../logger';
3231
import {
3332
Job,
3433
JobDescription,
@@ -305,8 +304,6 @@ export class SimpleScheduler<
305304
let state = JobState.Queued;
306305
let pingId = 0;
307306

308-
const logger = options.logger ? options.logger : new NullLogger();
309-
310307
// Create the input channel by having a filter.
311308
const input = new Subject<JsonValue>();
312309
input.pipe(
@@ -348,11 +345,6 @@ export class SimpleScheduler<
348345
state = this._updateState(message, state);
349346

350347
switch (message.kind) {
351-
case JobOutboundMessageKind.Log:
352-
const entry = message.entry;
353-
logger.log(entry.level, entry.message, entry);
354-
break;
355-
356348
case JobOutboundMessageKind.ChannelCreate: {
357349
const maybeSubject = channelsSubject.get(message.name);
358350
// If it doesn't exist or it's closed on the other end.

0 commit comments

Comments
 (0)