| Message ID | 20181030111916.7342-18-stephen@that.guru |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Add OpenAPI 3.0.0 REST API spec | expand |
I've now very lightly skimmed the preceeding commits to the yaml file. I don't really mind if it goes in, but I'd expect it to bitrot pretty quickly if there isn't one or more of the following: - real uptake of it - it being almost entirely computer-generated - automatic validation/regression testing of the spec against the code in e.g. tox. This commit makes me more uncomfortable, I think it's going to end up being more trouble than it's worth vs just keeping a file for each schema. The old ones should stay static, right? Anyway, it's not a hard NAK, but you should include jinja2 in a requirements file, unless it's pulled in by something else - and even then it wouldn't hurt to note it. Regards, Daniel Stephen Finucane <stephen@that.guru> writes: > OpenAPI doesn't appear to support versioning natively, suggesting > instead that separate documents are kept. Rather than doing this > manually, let's use a templating tool - Jinja2, in this case - to > generate these document for us from a single master document. > > Signed-off-by: Stephen Finucane <stephen@that.guru> > --- > .gitignore | 1 + > docs/api/schemas/generate_schema.py | 29 +++++++++++++++++ > .../schemas/{patchwork.yaml => patchwork.j2} | 31 +++++++++++++++++-- > 3 files changed, 59 insertions(+), 2 deletions(-) > create mode 100644 docs/api/schemas/generate_schema.py > rename docs/api/schemas/{patchwork.yaml => patchwork.j2} (98%) > > diff --git a/.gitignore b/.gitignore > index a33d1029..da53b382 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -44,6 +44,7 @@ htmlcov/ > > # Sphinx stuff > /docs/_build > +/docs/api/schemas/v* > > # Selenium test artifacts > /selenium.log > diff --git a/docs/api/schemas/generate_schema.py b/docs/api/schemas/generate_schema.py > new file mode 100644 > index 00000000..65d4ed82 > --- /dev/null > +++ b/docs/api/schemas/generate_schema.py > @@ -0,0 +1,29 @@ > +#!/usr/bin/env python3 > + > +import os > + > +import jinja2 > + > +ROOT_DIR = os.path.dirname(os.path.realpath(__file__)) > +VERSIONS = [(1, 0), (1, 1)] > + > + > +def generate_schema(): > + env = jinja2.Environment( > + loader=jinja2.FileSystemLoader(ROOT_DIR), > + trim_blocks=True, > + lstrip_blocks=True) > + template = env.get_template('patchwork.j2') > + > + for version in VERSIONS: > + version_dir = os.path.join(ROOT_DIR, 'v%d.%d' % version) > + > + if not os.path.exists(version_dir): > + os.mkdir(version_dir) > + > + with open(os.path.join(version_dir, 'patchwork.yaml'), 'wb') as fh: > + template.stream(version=version).dump(fh, encoding='utf-8') > + > + > +if __name__ == '__main__': > + generate_schema() > diff --git a/docs/api/schemas/patchwork.yaml b/docs/api/schemas/patchwork.j2 > similarity index 98% > rename from docs/api/schemas/patchwork.yaml > rename to docs/api/schemas/patchwork.j2 > index cf78a87f..752a9c5e 100644 > --- a/docs/api/schemas/patchwork.yaml > +++ b/docs/api/schemas/patchwork.j2 > @@ -1,3 +1,6 @@ > +{# You can obviously ignore the below when editing this template #} > +# DO NOT EDIT THIS FILE. It is generated from a template. Changes should be > +# proposed against the template. > openapi: '3.0.0' > info: > title: Patchwork API > @@ -9,7 +12,7 @@ info: > license: > name: GPL v2 License > url: https://www.gnu.org/licenses/gpl-2.0.html > - version: '1.1' > + version: '{{ "%d.%d"|format(*version) }}' > paths: > /api/: > get: > @@ -1137,11 +1140,13 @@ components: > type: string > format: uri > readOnly: true > +{% if version == '1.0' %} > web_url: > title: Web url > type: string > format: uri > readOnly: true > +{% endif %} > project: > $ref: '#/components/schemas/ProjectEmbedded' > name: > @@ -1256,11 +1261,13 @@ components: > title: ID > type: integer > readOnly: true > +{% if version > (1, 0) %} > web_url: > title: Web url > type: string > format: uri > readOnly: true > +{% endif %} > msgid: > title: Msgid > type: string > @@ -1302,11 +1309,13 @@ components: > type: string > format: uri > readOnly: true > +{% if version > (1, 0) %} > web_url: > title: Web url > type: string > format: uri > readOnly: true > +{% endif %} > project: > $ref: '#/components/schemas/ProjectEmbedded' > msgid: > @@ -1326,18 +1335,22 @@ components: > minLength: 1 > submitter: > $ref: '#/components/schemas/PersonEmbedded' > +{% if version > (1, 0) %} > mbox: > title: Mbox > type: string > format: uri > readOnly: true > +{% endif %} > series: > $ref: '#/components/schemas/SeriesEmbedded' > +{% if version > (1, 0) %} > comments: > title: Comments > type: string > format: uri > readOnly: true > +{% endif %} > CoverLetterDetail: > allOf: > - $ref: '#/components/schemas/CoverLetterList' > @@ -1506,11 +1519,13 @@ components: > type: string > format: uri > readOnly: true > +{% if version > (1, 0) %} > web_url: > title: Web url > type: string > format: uri > readOnly: true > +{% endif %} > project: > $ref: '#/components/schemas/ProjectEmbedded' > msgid: > @@ -1566,11 +1581,13 @@ components: > readOnly: true > series: > $ref: '#/components/schemas/SeriesEmbedded' > +{% if version > (1, 0) %} > comments: > title: Comments > type: string > format: uri > readOnly: true > +{% endif %} > check: > title: Check > type: string > @@ -1725,6 +1742,7 @@ components: > $ref: '#/components/schemas/UserEmbedded' > readOnly: true > uniqueItems: true > +{% if version > (1, 0) %} > subject_match: > title: Subject match > description: Regex to match the subject against if only part of emails > @@ -1735,6 +1753,7 @@ components: > type: string > readOnly: true > minLength: 1 > +{% endif %} > Series: > type: object > properties: > @@ -1747,11 +1766,13 @@ components: > type: string > format: uri > readOnly: true > +{% if version > (1, 0) %} > web_url: > title: Web url > type: string > format: uri > readOnly: true > +{% endif %} > project: > $ref: '#/components/schemas/ProjectEmbedded' > name: > @@ -1889,11 +1910,13 @@ components: > type: string > format: uri > readOnly: true > +{% if version > (1, 0) %} > web_url: > title: Web url > type: string > format: uri > readOnly: true > +{% endif %} > msgid: > title: Msgid > type: string > @@ -1926,11 +1949,13 @@ components: > type: string > format: uri > readOnly: true > +{% if version > (1, 0) %} > web_url: > title: Web url > type: string > format: uri > readOnly: true > +{% endif %} > msgid: > title: Msgid > type: string > @@ -2040,11 +2065,13 @@ components: > type: string > format: uri > readOnly: true > +{% if version > (1, 0) %} > web_url: > title: Web url > type: string > format: uri > readOnly: true > +{% endif %} > name: > title: Name > description: An optional name to associate with the series, e.g. "John's PCI > @@ -2177,4 +2204,4 @@ components: > type: string > readOnly: true > servers: > -- url: 'https://patchwork.ozlabs.org/api/1.1' > +- url: 'https://patchwork.ozlabs.org/api/{{ "%d.%d"|format(*version) }}' > -- > 2.17.2 > > _______________________________________________ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork
On Fri, 2018-11-09 at 00:30 +1100, Daniel Axtens wrote: > I've now very lightly skimmed the preceeding commits to the yaml file. > I don't really mind if it goes in, but I'd expect it to bitrot > pretty quickly if there isn't one or more of the following: > - real uptake of it > - it being almost entirely computer-generated > - automatic validation/regression testing of the spec against the code > in e.g. tox. Agreed. I have a series in the works to generate this using the openapi-core library [1][2]. My thinking is that we'll use the schema to validate things both server side and client side, starting with git- pw (and extending to pwclient once we decide how we're moving forward with that). This should make testing of clients a heck of a lot easier (currently I need to spin up a Patchwork instance or rely on unit tests) and it also opens the door for automatic client API code generation in the future. Having worked with git-pw for a while now, I think that's a increasingly good idea. > This commit makes me more uncomfortable, I think it's going to end up > being more trouble than it's worth vs just keeping a file for each > schema. The old ones should stay static, right? My thinking for this was to avoid duplication and avoid storing auto- generated code in version control. The documentation patches I plan to send later include a Sphinx extension to call the 'generate_schema' command locally before docs. Ditto for testing. I can store the generated files in-tree if this would be preferred though. > Anyway, it's not a hard NAK, but you should include jinja2 in a > requirements file, unless it's pulled in by something else - and even > then it wouldn't hurt to note it. Fair. Sphinx is pulling Jinja2 in but I can note this explicitly. Stephen [1] https://github.com/p1c2u/openapi-core [2] https://pypi.org/project/openapi-core/ > Regards, > Daniel > > Stephen Finucane <stephen@that.guru> writes: > > > OpenAPI doesn't appear to support versioning natively, suggesting > > instead that separate documents are kept. Rather than doing this > > manually, let's use a templating tool - Jinja2, in this case - to > > generate these document for us from a single master document. > > > > Signed-off-by: Stephen Finucane <stephen@that.guru> > > --- > > .gitignore | 1 + > > docs/api/schemas/generate_schema.py | 29 +++++++++++++++++ > > .../schemas/{patchwork.yaml => patchwork.j2} | 31 +++++++++++++++++-- > > 3 files changed, 59 insertions(+), 2 deletions(-) > > create mode 100644 docs/api/schemas/generate_schema.py > > rename docs/api/schemas/{patchwork.yaml => patchwork.j2} (98%) > > > > diff --git a/.gitignore b/.gitignore > > index a33d1029..da53b382 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -44,6 +44,7 @@ htmlcov/ > > > > # Sphinx stuff > > /docs/_build > > +/docs/api/schemas/v* > > > > # Selenium test artifacts > > /selenium.log > > diff --git a/docs/api/schemas/generate_schema.py b/docs/api/schemas/generate_schema.py > > new file mode 100644 > > index 00000000..65d4ed82 > > --- /dev/null > > +++ b/docs/api/schemas/generate_schema.py > > @@ -0,0 +1,29 @@ > > +#!/usr/bin/env python3 > > + > > +import os > > + > > +import jinja2 > > + > > +ROOT_DIR = os.path.dirname(os.path.realpath(__file__)) > > +VERSIONS = [(1, 0), (1, 1)] > > + > > + > > +def generate_schema(): > > + env = jinja2.Environment( > > + loader=jinja2.FileSystemLoader(ROOT_DIR), > > + trim_blocks=True, > > + lstrip_blocks=True) > > + template = env.get_template('patchwork.j2') > > + > > + for version in VERSIONS: > > + version_dir = os.path.join(ROOT_DIR, 'v%d.%d' % version) > > + > > + if not os.path.exists(version_dir): > > + os.mkdir(version_dir) > > + > > + with open(os.path.join(version_dir, 'patchwork.yaml'), 'wb') as fh: > > + template.stream(version=version).dump(fh, encoding='utf-8') > > + > > + > > +if __name__ == '__main__': > > + generate_schema() > > diff --git a/docs/api/schemas/patchwork.yaml b/docs/api/schemas/patchwork.j2 > > similarity index 98% > > rename from docs/api/schemas/patchwork.yaml > > rename to docs/api/schemas/patchwork.j2 > > index cf78a87f..752a9c5e 100644 > > --- a/docs/api/schemas/patchwork.yaml > > +++ b/docs/api/schemas/patchwork.j2 > > @@ -1,3 +1,6 @@ > > +{# You can obviously ignore the below when editing this template #} > > +# DO NOT EDIT THIS FILE. It is generated from a template. Changes should be > > +# proposed against the template. > > openapi: '3.0.0' > > info: > > title: Patchwork API > > @@ -9,7 +12,7 @@ info: > > license: > > name: GPL v2 License > > url: https://www.gnu.org/licenses/gpl-2.0.html > > - version: '1.1' > > + version: '{{ "%d.%d"|format(*version) }}' > > paths: > > /api/: > > get: > > @@ -1137,11 +1140,13 @@ components: > > type: string > > format: uri > > readOnly: true > > +{% if version == '1.0' %} > > web_url: > > title: Web url > > type: string > > format: uri > > readOnly: true > > +{% endif %} > > project: > > $ref: '#/components/schemas/ProjectEmbedded' > > name: > > @@ -1256,11 +1261,13 @@ components: > > title: ID > > type: integer > > readOnly: true > > +{% if version > (1, 0) %} > > web_url: > > title: Web url > > type: string > > format: uri > > readOnly: true > > +{% endif %} > > msgid: > > title: Msgid > > type: string > > @@ -1302,11 +1309,13 @@ components: > > type: string > > format: uri > > readOnly: true > > +{% if version > (1, 0) %} > > web_url: > > title: Web url > > type: string > > format: uri > > readOnly: true > > +{% endif %} > > project: > > $ref: '#/components/schemas/ProjectEmbedded' > > msgid: > > @@ -1326,18 +1335,22 @@ components: > > minLength: 1 > > submitter: > > $ref: '#/components/schemas/PersonEmbedded' > > +{% if version > (1, 0) %} > > mbox: > > title: Mbox > > type: string > > format: uri > > readOnly: true > > +{% endif %} > > series: > > $ref: '#/components/schemas/SeriesEmbedded' > > +{% if version > (1, 0) %} > > comments: > > title: Comments > > type: string > > format: uri > > readOnly: true > > +{% endif %} > > CoverLetterDetail: > > allOf: > > - $ref: '#/components/schemas/CoverLetterList' > > @@ -1506,11 +1519,13 @@ components: > > type: string > > format: uri > > readOnly: true > > +{% if version > (1, 0) %} > > web_url: > > title: Web url > > type: string > > format: uri > > readOnly: true > > +{% endif %} > > project: > > $ref: '#/components/schemas/ProjectEmbedded' > > msgid: > > @@ -1566,11 +1581,13 @@ components: > > readOnly: true > > series: > > $ref: '#/components/schemas/SeriesEmbedded' > > +{% if version > (1, 0) %} > > comments: > > title: Comments > > type: string > > format: uri > > readOnly: true > > +{% endif %} > > check: > > title: Check > > type: string > > @@ -1725,6 +1742,7 @@ components: > > $ref: '#/components/schemas/UserEmbedded' > > readOnly: true > > uniqueItems: true > > +{% if version > (1, 0) %} > > subject_match: > > title: Subject match > > description: Regex to match the subject against if only part of emails > > @@ -1735,6 +1753,7 @@ components: > > type: string > > readOnly: true > > minLength: 1 > > +{% endif %} > > Series: > > type: object > > properties: > > @@ -1747,11 +1766,13 @@ components: > > type: string > > format: uri > > readOnly: true > > +{% if version > (1, 0) %} > > web_url: > > title: Web url > > type: string > > format: uri > > readOnly: true > > +{% endif %} > > project: > > $ref: '#/components/schemas/ProjectEmbedded' > > name: > > @@ -1889,11 +1910,13 @@ components: > > type: string > > format: uri > > readOnly: true > > +{% if version > (1, 0) %} > > web_url: > > title: Web url > > type: string > > format: uri > > readOnly: true > > +{% endif %} > > msgid: > > title: Msgid > > type: string > > @@ -1926,11 +1949,13 @@ components: > > type: string > > format: uri > > readOnly: true > > +{% if version > (1, 0) %} > > web_url: > > title: Web url > > type: string > > format: uri > > readOnly: true > > +{% endif %} > > msgid: > > title: Msgid > > type: string > > @@ -2040,11 +2065,13 @@ components: > > type: string > > format: uri > > readOnly: true > > +{% if version > (1, 0) %} > > web_url: > > title: Web url > > type: string > > format: uri > > readOnly: true > > +{% endif %} > > name: > > title: Name > > description: An optional name to associate with the series, e.g. "John's PCI > > @@ -2177,4 +2204,4 @@ components: > > type: string > > readOnly: true > > servers: > > -- url: 'https://patchwork.ozlabs.org/api/1.1' > > +- url: 'https://patchwork.ozlabs.org/api/{{ "%d.%d"|format(*version) }}' > > -- > > 2.17.2 > > > > _______________________________________________ > > Patchwork mailing list > > Patchwork@lists.ozlabs.org > > https://lists.ozlabs.org/listinfo/patchwork
Stephen Finucane <stephen@that.guru> writes: > On Fri, 2018-11-09 at 00:30 +1100, Daniel Axtens wrote: >> I've now very lightly skimmed the preceeding commits to the yaml file. >> I don't really mind if it goes in, but I'd expect it to bitrot >> pretty quickly if there isn't one or more of the following: >> - real uptake of it >> - it being almost entirely computer-generated >> - automatic validation/regression testing of the spec against the code >> in e.g. tox. > > Agreed. I have a series in the works to generate this using the > openapi-core library [1][2]. My thinking is that we'll use the schema > to validate things both server side and client side, starting with git- > pw (and extending to pwclient once we decide how we're moving forward > with that). This should make testing of clients a heck of a lot easier > (currently I need to spin up a Patchwork instance or rely on unit > tests) and it also opens the door for automatic client API code > generation in the future. Having worked with git-pw for a while now, I > think that's a increasingly good idea. > >> This commit makes me more uncomfortable, I think it's going to end up >> being more trouble than it's worth vs just keeping a file for each >> schema. The old ones should stay static, right? > > My thinking for this was to avoid duplication and avoid storing auto- > generated code in version control. The documentation patches I plan to > send later include a Sphinx extension to call the 'generate_schema' > command locally before docs. Ditto for testing. I can store the > generated files in-tree if this would be preferred though. Well it's not really autogenerated if you have to do a bunch of manual editing to the generated file :) And I'm guessing you won't be able to auto-generate the fully templated file either? I also don't really find the idea of having API descriptions be quite similar between versions to be that big of a deal - or at least, not as big a deal as the added complexity of deduplicating via templating. My gut feeling is that you're making a rod for your own back with the added complexity - but I haven't got a good grasp on the grand scheme you have in mind, so maybe it works out simpler in the long run. Regards, Daniel > >> Anyway, it's not a hard NAK, but you should include jinja2 in a >> requirements file, unless it's pulled in by something else - and even >> then it wouldn't hurt to note it. > > Fair. Sphinx is pulling Jinja2 in but I can note this explicitly. > > Stephen > > [1] https://github.com/p1c2u/openapi-core > [2] https://pypi.org/project/openapi-core/ > >> Regards, >> Daniel >> >> Stephen Finucane <stephen@that.guru> writes: >> >> > OpenAPI doesn't appear to support versioning natively, suggesting >> > instead that separate documents are kept. Rather than doing this >> > manually, let's use a templating tool - Jinja2, in this case - to >> > generate these document for us from a single master document. >> > >> > Signed-off-by: Stephen Finucane <stephen@that.guru> >> > --- >> > .gitignore | 1 + >> > docs/api/schemas/generate_schema.py | 29 +++++++++++++++++ >> > .../schemas/{patchwork.yaml => patchwork.j2} | 31 +++++++++++++++++-- >> > 3 files changed, 59 insertions(+), 2 deletions(-) >> > create mode 100644 docs/api/schemas/generate_schema.py >> > rename docs/api/schemas/{patchwork.yaml => patchwork.j2} (98%) >> > >> > diff --git a/.gitignore b/.gitignore >> > index a33d1029..da53b382 100644 >> > --- a/.gitignore >> > +++ b/.gitignore >> > @@ -44,6 +44,7 @@ htmlcov/ >> > >> > # Sphinx stuff >> > /docs/_build >> > +/docs/api/schemas/v* >> > >> > # Selenium test artifacts >> > /selenium.log >> > diff --git a/docs/api/schemas/generate_schema.py b/docs/api/schemas/generate_schema.py >> > new file mode 100644 >> > index 00000000..65d4ed82 >> > --- /dev/null >> > +++ b/docs/api/schemas/generate_schema.py >> > @@ -0,0 +1,29 @@ >> > +#!/usr/bin/env python3 >> > + >> > +import os >> > + >> > +import jinja2 >> > + >> > +ROOT_DIR = os.path.dirname(os.path.realpath(__file__)) >> > +VERSIONS = [(1, 0), (1, 1)] >> > + >> > + >> > +def generate_schema(): >> > + env = jinja2.Environment( >> > + loader=jinja2.FileSystemLoader(ROOT_DIR), >> > + trim_blocks=True, >> > + lstrip_blocks=True) >> > + template = env.get_template('patchwork.j2') >> > + >> > + for version in VERSIONS: >> > + version_dir = os.path.join(ROOT_DIR, 'v%d.%d' % version) >> > + >> > + if not os.path.exists(version_dir): >> > + os.mkdir(version_dir) >> > + >> > + with open(os.path.join(version_dir, 'patchwork.yaml'), 'wb') as fh: >> > + template.stream(version=version).dump(fh, encoding='utf-8') >> > + >> > + >> > +if __name__ == '__main__': >> > + generate_schema() >> > diff --git a/docs/api/schemas/patchwork.yaml b/docs/api/schemas/patchwork.j2 >> > similarity index 98% >> > rename from docs/api/schemas/patchwork.yaml >> > rename to docs/api/schemas/patchwork.j2 >> > index cf78a87f..752a9c5e 100644 >> > --- a/docs/api/schemas/patchwork.yaml >> > +++ b/docs/api/schemas/patchwork.j2 >> > @@ -1,3 +1,6 @@ >> > +{# You can obviously ignore the below when editing this template #} >> > +# DO NOT EDIT THIS FILE. It is generated from a template. Changes should be >> > +# proposed against the template. >> > openapi: '3.0.0' >> > info: >> > title: Patchwork API >> > @@ -9,7 +12,7 @@ info: >> > license: >> > name: GPL v2 License >> > url: https://www.gnu.org/licenses/gpl-2.0.html >> > - version: '1.1' >> > + version: '{{ "%d.%d"|format(*version) }}' >> > paths: >> > /api/: >> > get: >> > @@ -1137,11 +1140,13 @@ components: >> > type: string >> > format: uri >> > readOnly: true >> > +{% if version == '1.0' %} >> > web_url: >> > title: Web url >> > type: string >> > format: uri >> > readOnly: true >> > +{% endif %} >> > project: >> > $ref: '#/components/schemas/ProjectEmbedded' >> > name: >> > @@ -1256,11 +1261,13 @@ components: >> > title: ID >> > type: integer >> > readOnly: true >> > +{% if version > (1, 0) %} >> > web_url: >> > title: Web url >> > type: string >> > format: uri >> > readOnly: true >> > +{% endif %} >> > msgid: >> > title: Msgid >> > type: string >> > @@ -1302,11 +1309,13 @@ components: >> > type: string >> > format: uri >> > readOnly: true >> > +{% if version > (1, 0) %} >> > web_url: >> > title: Web url >> > type: string >> > format: uri >> > readOnly: true >> > +{% endif %} >> > project: >> > $ref: '#/components/schemas/ProjectEmbedded' >> > msgid: >> > @@ -1326,18 +1335,22 @@ components: >> > minLength: 1 >> > submitter: >> > $ref: '#/components/schemas/PersonEmbedded' >> > +{% if version > (1, 0) %} >> > mbox: >> > title: Mbox >> > type: string >> > format: uri >> > readOnly: true >> > +{% endif %} >> > series: >> > $ref: '#/components/schemas/SeriesEmbedded' >> > +{% if version > (1, 0) %} >> > comments: >> > title: Comments >> > type: string >> > format: uri >> > readOnly: true >> > +{% endif %} >> > CoverLetterDetail: >> > allOf: >> > - $ref: '#/components/schemas/CoverLetterList' >> > @@ -1506,11 +1519,13 @@ components: >> > type: string >> > format: uri >> > readOnly: true >> > +{% if version > (1, 0) %} >> > web_url: >> > title: Web url >> > type: string >> > format: uri >> > readOnly: true >> > +{% endif %} >> > project: >> > $ref: '#/components/schemas/ProjectEmbedded' >> > msgid: >> > @@ -1566,11 +1581,13 @@ components: >> > readOnly: true >> > series: >> > $ref: '#/components/schemas/SeriesEmbedded' >> > +{% if version > (1, 0) %} >> > comments: >> > title: Comments >> > type: string >> > format: uri >> > readOnly: true >> > +{% endif %} >> > check: >> > title: Check >> > type: string >> > @@ -1725,6 +1742,7 @@ components: >> > $ref: '#/components/schemas/UserEmbedded' >> > readOnly: true >> > uniqueItems: true >> > +{% if version > (1, 0) %} >> > subject_match: >> > title: Subject match >> > description: Regex to match the subject against if only part of emails >> > @@ -1735,6 +1753,7 @@ components: >> > type: string >> > readOnly: true >> > minLength: 1 >> > +{% endif %} >> > Series: >> > type: object >> > properties: >> > @@ -1747,11 +1766,13 @@ components: >> > type: string >> > format: uri >> > readOnly: true >> > +{% if version > (1, 0) %} >> > web_url: >> > title: Web url >> > type: string >> > format: uri >> > readOnly: true >> > +{% endif %} >> > project: >> > $ref: '#/components/schemas/ProjectEmbedded' >> > name: >> > @@ -1889,11 +1910,13 @@ components: >> > type: string >> > format: uri >> > readOnly: true >> > +{% if version > (1, 0) %} >> > web_url: >> > title: Web url >> > type: string >> > format: uri >> > readOnly: true >> > +{% endif %} >> > msgid: >> > title: Msgid >> > type: string >> > @@ -1926,11 +1949,13 @@ components: >> > type: string >> > format: uri >> > readOnly: true >> > +{% if version > (1, 0) %} >> > web_url: >> > title: Web url >> > type: string >> > format: uri >> > readOnly: true >> > +{% endif %} >> > msgid: >> > title: Msgid >> > type: string >> > @@ -2040,11 +2065,13 @@ components: >> > type: string >> > format: uri >> > readOnly: true >> > +{% if version > (1, 0) %} >> > web_url: >> > title: Web url >> > type: string >> > format: uri >> > readOnly: true >> > +{% endif %} >> > name: >> > title: Name >> > description: An optional name to associate with the series, e.g. "John's PCI >> > @@ -2177,4 +2204,4 @@ components: >> > type: string >> > readOnly: true >> > servers: >> > -- url: 'https://patchwork.ozlabs.org/api/1.1' >> > +- url: 'https://patchwork.ozlabs.org/api/{{ "%d.%d"|format(*version) }}' >> > -- >> > 2.17.2 >> > >> > _______________________________________________ >> > Patchwork mailing list >> > Patchwork@lists.ozlabs.org >> > https://lists.ozlabs.org/listinfo/patchwork
On Sun, 2018-11-11 at 10:36 +1100, Daniel Axtens wrote: > Stephen Finucane <stephen@that.guru> writes: > > > On Fri, 2018-11-09 at 00:30 +1100, Daniel Axtens wrote: > > > I've now very lightly skimmed the preceeding commits to the yaml file. > > > I don't really mind if it goes in, but I'd expect it to bitrot > > > pretty quickly if there isn't one or more of the following: > > > - real uptake of it > > > - it being almost entirely computer-generated > > > - automatic validation/regression testing of the spec against the code > > > in e.g. tox. > > > > Agreed. I have a series in the works to generate this using the > > openapi-core library [1][2]. My thinking is that we'll use the schema > > to validate things both server side and client side, starting with git- > > pw (and extending to pwclient once we decide how we're moving forward > > with that). This should make testing of clients a heck of a lot easier > > (currently I need to spin up a Patchwork instance or rely on unit > > tests) and it also opens the door for automatic client API code > > generation in the future. Having worked with git-pw for a while now, I > > think that's a increasingly good idea. > > > > > This commit makes me more uncomfortable, I think it's going to end up > > > being more trouble than it's worth vs just keeping a file for each > > > schema. The old ones should stay static, right? > > > > My thinking for this was to avoid duplication and avoid storing auto- > > generated code in version control. The documentation patches I plan to > > send later include a Sphinx extension to call the 'generate_schema' > > command locally before docs. Ditto for testing. I can store the > > generated files in-tree if this would be preferred though. > > Well it's not really autogenerated if you have to do a bunch of manual > editing to the generated file :) And I'm guessing you won't be able to > auto-generate the fully templated file either? Ah, I meant the template was there to avoid duplication and I wasn't storing the output of the 'generate_schema' call because it was auto- generated. It's not possible to generate the schema automatically, with or without templates, because the tooling just isn't there yet. > I also don't really find the idea of having API descriptions be quite > similar between versions to be that big of a deal - or at least, not as > big a deal as the added complexity of deduplicating via templating. > > My gut feeling is that you're making a rod for your own back with the > added complexity - but I haven't got a good grasp on the grand scheme > you have in mind, so maybe it works out simpler in the long run. I do have an additional use for Jinja2, in the form of including the sample calls/responses introduced in that other patch series. Let's stick with this for now, until I have that fleshed out. If it turns out we can get away without Jinja2 for this, we can drop the templating and go with static. Sound fair? Stephen > Regards, > Daniel > > > > Anyway, it's not a hard NAK, but you should include jinja2 in a > > > requirements file, unless it's pulled in by something else - and even > > > then it wouldn't hurt to note it. > > > > Fair. Sphinx is pulling Jinja2 in but I can note this explicitly. > > > > Stephen > > > > [1] https://github.com/p1c2u/openapi-core > > [2] https://pypi.org/project/openapi-core/ > > > > > Regards, > > > Daniel > > > > > > Stephen Finucane <stephen@that.guru> writes: > > > > > > > OpenAPI doesn't appear to support versioning natively, suggesting > > > > instead that separate documents are kept. Rather than doing this > > > > manually, let's use a templating tool - Jinja2, in this case - to > > > > generate these document for us from a single master document. > > > > > > > > Signed-off-by: Stephen Finucane <stephen@that.guru> > > > > --- > > > > .gitignore | 1 + > > > > docs/api/schemas/generate_schema.py | 29 +++++++++++++++++ > > > > .../schemas/{patchwork.yaml => patchwork.j2} | 31 +++++++++++++++++-- > > > > 3 files changed, 59 insertions(+), 2 deletions(-) > > > > create mode 100644 docs/api/schemas/generate_schema.py > > > > rename docs/api/schemas/{patchwork.yaml => patchwork.j2} (98%) > > > > > > > > diff --git a/.gitignore b/.gitignore > > > > index a33d1029..da53b382 100644 > > > > --- a/.gitignore > > > > +++ b/.gitignore > > > > @@ -44,6 +44,7 @@ htmlcov/ > > > > > > > > # Sphinx stuff > > > > /docs/_build > > > > +/docs/api/schemas/v* > > > > > > > > # Selenium test artifacts > > > > /selenium.log > > > > diff --git a/docs/api/schemas/generate_schema.py b/docs/api/schemas/generate_schema.py > > > > new file mode 100644 > > > > index 00000000..65d4ed82 > > > > --- /dev/null > > > > +++ b/docs/api/schemas/generate_schema.py > > > > @@ -0,0 +1,29 @@ > > > > +#!/usr/bin/env python3 > > > > + > > > > +import os > > > > + > > > > +import jinja2 > > > > + > > > > +ROOT_DIR = os.path.dirname(os.path.realpath(__file__)) > > > > +VERSIONS = [(1, 0), (1, 1)] > > > > + > > > > + > > > > +def generate_schema(): > > > > + env = jinja2.Environment( > > > > + loader=jinja2.FileSystemLoader(ROOT_DIR), > > > > + trim_blocks=True, > > > > + lstrip_blocks=True) > > > > + template = env.get_template('patchwork.j2') > > > > + > > > > + for version in VERSIONS: > > > > + version_dir = os.path.join(ROOT_DIR, 'v%d.%d' % version) > > > > + > > > > + if not os.path.exists(version_dir): > > > > + os.mkdir(version_dir) > > > > + > > > > + with open(os.path.join(version_dir, 'patchwork.yaml'), 'wb') as fh: > > > > + template.stream(version=version).dump(fh, encoding='utf-8') > > > > + > > > > + > > > > +if __name__ == '__main__': > > > > + generate_schema() > > > > diff --git a/docs/api/schemas/patchwork.yaml b/docs/api/schemas/patchwork.j2 > > > > similarity index 98% > > > > rename from docs/api/schemas/patchwork.yaml > > > > rename to docs/api/schemas/patchwork.j2 > > > > index cf78a87f..752a9c5e 100644 > > > > --- a/docs/api/schemas/patchwork.yaml > > > > +++ b/docs/api/schemas/patchwork.j2 > > > > @@ -1,3 +1,6 @@ > > > > +{# You can obviously ignore the below when editing this template #} > > > > +# DO NOT EDIT THIS FILE. It is generated from a template. Changes should be > > > > +# proposed against the template. > > > > openapi: '3.0.0' > > > > info: > > > > title: Patchwork API > > > > @@ -9,7 +12,7 @@ info: > > > > license: > > > > name: GPL v2 License > > > > url: https://www.gnu.org/licenses/gpl-2.0.html > > > > - version: '1.1' > > > > + version: '{{ "%d.%d"|format(*version) }}' > > > > paths: > > > > /api/: > > > > get: > > > > @@ -1137,11 +1140,13 @@ components: > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% if version == '1.0' %} > > > > web_url: > > > > title: Web url > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% endif %} > > > > project: > > > > $ref: '#/components/schemas/ProjectEmbedded' > > > > name: > > > > @@ -1256,11 +1261,13 @@ components: > > > > title: ID > > > > type: integer > > > > readOnly: true > > > > +{% if version > (1, 0) %} > > > > web_url: > > > > title: Web url > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% endif %} > > > > msgid: > > > > title: Msgid > > > > type: string > > > > @@ -1302,11 +1309,13 @@ components: > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% if version > (1, 0) %} > > > > web_url: > > > > title: Web url > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% endif %} > > > > project: > > > > $ref: '#/components/schemas/ProjectEmbedded' > > > > msgid: > > > > @@ -1326,18 +1335,22 @@ components: > > > > minLength: 1 > > > > submitter: > > > > $ref: '#/components/schemas/PersonEmbedded' > > > > +{% if version > (1, 0) %} > > > > mbox: > > > > title: Mbox > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% endif %} > > > > series: > > > > $ref: '#/components/schemas/SeriesEmbedded' > > > > +{% if version > (1, 0) %} > > > > comments: > > > > title: Comments > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% endif %} > > > > CoverLetterDetail: > > > > allOf: > > > > - $ref: '#/components/schemas/CoverLetterList' > > > > @@ -1506,11 +1519,13 @@ components: > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% if version > (1, 0) %} > > > > web_url: > > > > title: Web url > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% endif %} > > > > project: > > > > $ref: '#/components/schemas/ProjectEmbedded' > > > > msgid: > > > > @@ -1566,11 +1581,13 @@ components: > > > > readOnly: true > > > > series: > > > > $ref: '#/components/schemas/SeriesEmbedded' > > > > +{% if version > (1, 0) %} > > > > comments: > > > > title: Comments > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% endif %} > > > > check: > > > > title: Check > > > > type: string > > > > @@ -1725,6 +1742,7 @@ components: > > > > $ref: '#/components/schemas/UserEmbedded' > > > > readOnly: true > > > > uniqueItems: true > > > > +{% if version > (1, 0) %} > > > > subject_match: > > > > title: Subject match > > > > description: Regex to match the subject against if only part of emails > > > > @@ -1735,6 +1753,7 @@ components: > > > > type: string > > > > readOnly: true > > > > minLength: 1 > > > > +{% endif %} > > > > Series: > > > > type: object > > > > properties: > > > > @@ -1747,11 +1766,13 @@ components: > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% if version > (1, 0) %} > > > > web_url: > > > > title: Web url > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% endif %} > > > > project: > > > > $ref: '#/components/schemas/ProjectEmbedded' > > > > name: > > > > @@ -1889,11 +1910,13 @@ components: > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% if version > (1, 0) %} > > > > web_url: > > > > title: Web url > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% endif %} > > > > msgid: > > > > title: Msgid > > > > type: string > > > > @@ -1926,11 +1949,13 @@ components: > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% if version > (1, 0) %} > > > > web_url: > > > > title: Web url > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% endif %} > > > > msgid: > > > > title: Msgid > > > > type: string > > > > @@ -2040,11 +2065,13 @@ components: > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% if version > (1, 0) %} > > > > web_url: > > > > title: Web url > > > > type: string > > > > format: uri > > > > readOnly: true > > > > +{% endif %} > > > > name: > > > > title: Name > > > > description: An optional name to associate with the series, e.g. "John's PCI > > > > @@ -2177,4 +2204,4 @@ components: > > > > type: string > > > > readOnly: true > > > > servers: > > > > -- url: 'https://patchwork.ozlabs.org/api/1.1' > > > > +- url: 'https://patchwork.ozlabs.org/api/{{ "%d.%d"|format(*version) }}' > > > > -- > > > > 2.17.2 > > > > > > > > _______________________________________________ > > > > Patchwork mailing list > > > > Patchwork@lists.ozlabs.org > > > > https://lists.ozlabs.org/listinfo/patchwork
diff --git a/.gitignore b/.gitignore index a33d1029..da53b382 100644 --- a/.gitignore +++ b/.gitignore @@ -44,6 +44,7 @@ htmlcov/ # Sphinx stuff /docs/_build +/docs/api/schemas/v* # Selenium test artifacts /selenium.log diff --git a/docs/api/schemas/generate_schema.py b/docs/api/schemas/generate_schema.py new file mode 100644 index 00000000..65d4ed82 --- /dev/null +++ b/docs/api/schemas/generate_schema.py @@ -0,0 +1,29 @@ +#!/usr/bin/env python3 + +import os + +import jinja2 + +ROOT_DIR = os.path.dirname(os.path.realpath(__file__)) +VERSIONS = [(1, 0), (1, 1)] + + +def generate_schema(): + env = jinja2.Environment( + loader=jinja2.FileSystemLoader(ROOT_DIR), + trim_blocks=True, + lstrip_blocks=True) + template = env.get_template('patchwork.j2') + + for version in VERSIONS: + version_dir = os.path.join(ROOT_DIR, 'v%d.%d' % version) + + if not os.path.exists(version_dir): + os.mkdir(version_dir) + + with open(os.path.join(version_dir, 'patchwork.yaml'), 'wb') as fh: + template.stream(version=version).dump(fh, encoding='utf-8') + + +if __name__ == '__main__': + generate_schema() diff --git a/docs/api/schemas/patchwork.yaml b/docs/api/schemas/patchwork.j2 similarity index 98% rename from docs/api/schemas/patchwork.yaml rename to docs/api/schemas/patchwork.j2 index cf78a87f..752a9c5e 100644 --- a/docs/api/schemas/patchwork.yaml +++ b/docs/api/schemas/patchwork.j2 @@ -1,3 +1,6 @@ +{# You can obviously ignore the below when editing this template #} +# DO NOT EDIT THIS FILE. It is generated from a template. Changes should be +# proposed against the template. openapi: '3.0.0' info: title: Patchwork API @@ -9,7 +12,7 @@ info: license: name: GPL v2 License url: https://www.gnu.org/licenses/gpl-2.0.html - version: '1.1' + version: '{{ "%d.%d"|format(*version) }}' paths: /api/: get: @@ -1137,11 +1140,13 @@ components: type: string format: uri readOnly: true +{% if version == '1.0' %} web_url: title: Web url type: string format: uri readOnly: true +{% endif %} project: $ref: '#/components/schemas/ProjectEmbedded' name: @@ -1256,11 +1261,13 @@ components: title: ID type: integer readOnly: true +{% if version > (1, 0) %} web_url: title: Web url type: string format: uri readOnly: true +{% endif %} msgid: title: Msgid type: string @@ -1302,11 +1309,13 @@ components: type: string format: uri readOnly: true +{% if version > (1, 0) %} web_url: title: Web url type: string format: uri readOnly: true +{% endif %} project: $ref: '#/components/schemas/ProjectEmbedded' msgid: @@ -1326,18 +1335,22 @@ components: minLength: 1 submitter: $ref: '#/components/schemas/PersonEmbedded' +{% if version > (1, 0) %} mbox: title: Mbox type: string format: uri readOnly: true +{% endif %} series: $ref: '#/components/schemas/SeriesEmbedded' +{% if version > (1, 0) %} comments: title: Comments type: string format: uri readOnly: true +{% endif %} CoverLetterDetail: allOf: - $ref: '#/components/schemas/CoverLetterList' @@ -1506,11 +1519,13 @@ components: type: string format: uri readOnly: true +{% if version > (1, 0) %} web_url: title: Web url type: string format: uri readOnly: true +{% endif %} project: $ref: '#/components/schemas/ProjectEmbedded' msgid: @@ -1566,11 +1581,13 @@ components: readOnly: true series: $ref: '#/components/schemas/SeriesEmbedded' +{% if version > (1, 0) %} comments: title: Comments type: string format: uri readOnly: true +{% endif %} check: title: Check type: string @@ -1725,6 +1742,7 @@ components: $ref: '#/components/schemas/UserEmbedded' readOnly: true uniqueItems: true +{% if version > (1, 0) %} subject_match: title: Subject match description: Regex to match the subject against if only part of emails @@ -1735,6 +1753,7 @@ components: type: string readOnly: true minLength: 1 +{% endif %} Series: type: object properties: @@ -1747,11 +1766,13 @@ components: type: string format: uri readOnly: true +{% if version > (1, 0) %} web_url: title: Web url type: string format: uri readOnly: true +{% endif %} project: $ref: '#/components/schemas/ProjectEmbedded' name: @@ -1889,11 +1910,13 @@ components: type: string format: uri readOnly: true +{% if version > (1, 0) %} web_url: title: Web url type: string format: uri readOnly: true +{% endif %} msgid: title: Msgid type: string @@ -1926,11 +1949,13 @@ components: type: string format: uri readOnly: true +{% if version > (1, 0) %} web_url: title: Web url type: string format: uri readOnly: true +{% endif %} msgid: title: Msgid type: string @@ -2040,11 +2065,13 @@ components: type: string format: uri readOnly: true +{% if version > (1, 0) %} web_url: title: Web url type: string format: uri readOnly: true +{% endif %} name: title: Name description: An optional name to associate with the series, e.g. "John's PCI @@ -2177,4 +2204,4 @@ components: type: string readOnly: true servers: -- url: 'https://patchwork.ozlabs.org/api/1.1' +- url: 'https://patchwork.ozlabs.org/api/{{ "%d.%d"|format(*version) }}'
OpenAPI doesn't appear to support versioning natively, suggesting instead that separate documents are kept. Rather than doing this manually, let's use a templating tool - Jinja2, in this case - to generate these document for us from a single master document. Signed-off-by: Stephen Finucane <stephen@that.guru> --- .gitignore | 1 + docs/api/schemas/generate_schema.py | 29 +++++++++++++++++ .../schemas/{patchwork.yaml => patchwork.j2} | 31 +++++++++++++++++-- 3 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 docs/api/schemas/generate_schema.py rename docs/api/schemas/{patchwork.yaml => patchwork.j2} (98%)