-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update source-gnews manifest to use inline stream schemas #20405
Changes from all commits
5d5e5a5
1678bb2
febe06f
10d7558
e39c2d9
c7620fe
a8b6946
4a351c1
56b0895
a69fbb6
8221d2e
f9f1700
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,107 @@ | ||
version: "0.1.0" | ||
|
||
schemas: | ||
search_stream_schema: | ||
type: object | ||
properties: | ||
title: | ||
type: | ||
- "null" | ||
- string | ||
description: The main title of the article. | ||
description: | ||
type: | ||
- "null" | ||
- string | ||
description: The small paragraph under the title. | ||
content: | ||
type: | ||
- "null" | ||
- string | ||
description: All the content of the article. | ||
url: | ||
type: | ||
- "null" | ||
- string | ||
description: The URL of the article. | ||
image: | ||
type: | ||
- "null" | ||
- string | ||
description: The main image of the article. | ||
publishedAt: | ||
type: | ||
- "null" | ||
- string | ||
description: | ||
The date of publication of the article. The date is always in the | ||
UTC time zone. | ||
source: | ||
type: | ||
- "null" | ||
- object | ||
properties: | ||
name: | ||
type: | ||
- "null" | ||
- string | ||
description: The name of the source. | ||
url: | ||
type: | ||
- "null" | ||
- string | ||
description: The home page of the source. | ||
top_headlines_stream_schema: | ||
type: object | ||
properties: | ||
title: | ||
type: | ||
- "null" | ||
- string | ||
description: The main title of the article. | ||
description: | ||
type: | ||
- "null" | ||
- string | ||
description: The small paragraph under the title. | ||
content: | ||
type: | ||
- "null" | ||
- string | ||
description: All the content of the article. | ||
url: | ||
type: | ||
- "null" | ||
- string | ||
description: The URL of the article. | ||
image: | ||
type: | ||
- "null" | ||
- string | ||
description: The main image of the article. | ||
publishedAt: | ||
type: | ||
- "null" | ||
- string | ||
description: | ||
The date of publication of the article. The date is always in the | ||
UTC time zone. | ||
source: | ||
type: | ||
- "null" | ||
- object | ||
properties: | ||
name: | ||
type: | ||
- "null" | ||
- string | ||
description: The name of the source. | ||
url: | ||
type: | ||
- "null" | ||
- string | ||
description: The home page of the source. | ||
|
||
definitions: | ||
selector: | ||
extractor: | ||
|
@@ -44,7 +146,12 @@ definitions: | |
nullable: "{{ ','.join(config['nullable']) }}" | ||
from: "{{ stream_slice['start_time'] }}" | ||
to: "{{ stream_slice['end_time'] }}" | ||
schema_loader: | ||
type: InlineSchemaLoader | ||
search_stream: | ||
schema_loader: | ||
$ref: "*ref(definitions.schema_loader)" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fwiw I would find it more understandable if this said
instead of the ref There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. |
||
schema: "*ref(schemas.search_stream_schema)" | ||
$options: | ||
name: "search" | ||
primary_key: "url" | ||
|
@@ -61,6 +168,9 @@ definitions: | |
in: "{{ ','.join(config['in']) }}" | ||
sortby: "{{ config['sortby'] }}" | ||
top_headlines_stream: | ||
schema_loader: | ||
$ref: "*ref(definitions.schema_loader)" | ||
schema: "*ref(schemas.top_headlines_stream_schema)" | ||
$options: | ||
name: "top_headlines" | ||
primary_key: "url" | ||
|
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, but can we move the schemas to the bottom of the file? i think the definitions and streams are likely to be the parts of the manifest that change most, so it's more intuitive to have them at the top.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah sorry you are correct and my mistake. I thought we did some preparsing to allow for forward references even if defined after, but doesn't seem like we do, so it's fine to have this at the top.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries (sorry, deleted my comment because I thought maybe I'd spoken prematurely and was looking more closely at the issue). Going to write up an issue for updating the parser to handle forward references, since I do think that will make the yaml more readable.