f2e5ddda29
Patch Set 5:
(2 comments)
Patch-set: 5
Attention: {"person_ident":"Gerrit User 11604 \u003c11604@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"REMOVE","reason":"\u003cGERRIT_ACCOUNT_11604\u003e replied on the change"}
Attention: {"person_ident":"Gerrit User 15334 \u003c15334@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"ADD","reason":"\u003cGERRIT_ACCOUNT_11604\u003e replied on the change"}
403 lines
20 KiB
Plaintext
403 lines
20 KiB
Plaintext
{
|
|
"comments": [
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "d5480c77_910c887f",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 27900
|
|
},
|
|
"writtenOn": "2024-02-22T12:41:26Z",
|
|
"side": 1,
|
|
"message": "+1 from me\n\nThere is a bunch of schemas that I created manually for the things missing in the source code. It would be great if we could not only add schemas, but directly fill them and operations with usable descriptions that flow into the spec. With the new sphinx-extension this would completely obsolete the api-ref, otherwise spec generator reads rendered html to extract descriptions from there (but this is not great). Keeping that human activity is unnecessary and error prone as we all know.",
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "e08fc758_3a1ca21b",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 15334
|
|
},
|
|
"writtenOn": "2024-02-29T14:44:04Z",
|
|
"side": 1,
|
|
"message": "I agree. We currently have no `title` or `description` fields. We should add those are part of this effort. I would like us to get to the point where there would be zero human input needed to generate a spec.",
|
|
"parentUuid": "d5480c77_910c887f",
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "d9cc2113_ffee09ae",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 11604
|
|
},
|
|
"writtenOn": "2024-03-27T12:20:36Z",
|
|
"side": 1,
|
|
"message": "im +1 on the core concept\nsome comments inline",
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "9cb87aa8_9a137937",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 38,
|
|
"author": {
|
|
"id": 11604
|
|
},
|
|
"writtenOn": "2024-03-27T12:20:36Z",
|
|
"side": 1,
|
|
"message": "by the way i would prefer if we avoid the annomous link style if we have mulitple links before the definitions.\n\nits too easy for them to get reorded ectra so can we go back to the named liks style\n```\n`CloudFlare`_\n\n.. _CloudFlare: https://blog.cloudflare.com/open-api-transition\n```\n\nim fine with the annoums styple if there is no other link between the the usage and the defintion but even then i generally prefer the older style.\n\ndocumented in https://sublime-and-sphinx-guide.readthedocs.io/en/latest/references.html#links-to-external-web-pages\nand here https://docutils.sourceforge.io/docs/user/rst/quickref.html#inline-markup",
|
|
"range": {
|
|
"startLine": 38,
|
|
"startChar": 19,
|
|
"endLine": 38,
|
|
"endChar": 49
|
|
},
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "4da5e131_7f9b8b81",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 38,
|
|
"author": {
|
|
"id": 15334
|
|
},
|
|
"writtenOn": "2024-03-27T17:13:52Z",
|
|
"side": 1,
|
|
"message": "I make sure to always place them at the bottom of the section they are used in, so this is rarely if ever an issue for me.",
|
|
"parentUuid": "9cb87aa8_9a137937",
|
|
"range": {
|
|
"startLine": 38,
|
|
"startChar": 19,
|
|
"endLine": 38,
|
|
"endChar": 49
|
|
},
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "f5006b74_e1d0bc78",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 62,
|
|
"author": {
|
|
"id": 11604
|
|
},
|
|
"writtenOn": "2024-03-27T12:20:36Z",
|
|
"side": 1,
|
|
"message": "^ that is a more likely thing the we might like although we currently are using jsonschema so hopefuly that is not an issue.\n\ni would like however to have that option and maybe even premetivly move if it would symplfy our codbase, that just does nto have to be done in this spec",
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "5bd1d0e0_b13f2477",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 62,
|
|
"author": {
|
|
"id": 15334
|
|
},
|
|
"writtenOn": "2024-03-27T17:13:52Z",
|
|
"side": 1,
|
|
"message": "I didn\u0027t mean replacing jsonschema. I meant replacing webob or routes (the latter of which is no longer actively maintained, and the former of which is barely maintained from the looks of things)",
|
|
"parentUuid": "f5006b74_e1d0bc78",
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "c80d3fd3_89b48a95",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 62,
|
|
"author": {
|
|
"id": 11604
|
|
},
|
|
"writtenOn": "2024-04-07T04:50:02Z",
|
|
"side": 1,
|
|
"message": "oh ok for what its worth i would prefer to move to flask or one fo the more moderen api frameworks but ya this could help with that at some point",
|
|
"parentUuid": "5bd1d0e0_b13f2477",
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "61c12355_2001447c",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 79,
|
|
"author": {
|
|
"id": 27900
|
|
},
|
|
"writtenOn": "2024-02-29T09:48:16Z",
|
|
"side": 1,
|
|
"message": "I think more reasonable would be to start with schemas already defined in openstack-codegenerator project since they are already mostly reviewed by human for sanity and address document bugs, microversions and actions. This can save quote lot of time.\n\nImportant: openstack-codegenerator does not currently have schemas for deprecated methods",
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "6dea2f3c_9a8a94b2",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 79,
|
|
"author": {
|
|
"id": 15334
|
|
},
|
|
"writtenOn": "2024-02-29T14:44:04Z",
|
|
"side": 1,
|
|
"message": "\u003e I think more reasonable would be to start with schemas already defined in openstack-codegenerator project since they are already mostly reviewed by human for sanity and address document bugs, microversions and actions. This can save quote lot of time.\n\nNot a bad idea.\n\n\u003e Important: openstack-codegenerator does not currently have schemas for deprecated methods\n\nOkay. We don\u0027t want to add these for removed (`HTTP 410 (Gone)`) APIs. We should have them for deprecated APIs though.",
|
|
"parentUuid": "61c12355_2001447c",
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "b7f7acf1_c21a6237",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 79,
|
|
"author": {
|
|
"id": 11604
|
|
},
|
|
"writtenOn": "2024-03-27T12:20:36Z",
|
|
"side": 1,
|
|
"message": "starting with those is fine but we will need to review them again when adding them to the nova repo for correctness.\n\ni agree that doing this for remvoed api like nova network is not somethign we should do. deprecated apis woudl be a low priority but dependign on the continued usage there may be some value so as long a we have not moved the api but just deprecated some usage of the api i.e. the force parmaters for live migration we still proably would want validatidtion for the deprecated microverions.\n\nthat would be a lower priorty for me then covering the actully supproted parts.\nso i would conder validateion fo deprectated functionality in older microversion to be a strech goal.",
|
|
"parentUuid": "6dea2f3c_9a8a94b2",
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "4ed2b6d8_35edf5b6",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 79,
|
|
"author": {
|
|
"id": 15334
|
|
},
|
|
"writtenOn": "2024-03-27T17:13:52Z",
|
|
"side": 1,
|
|
"message": "\u003e starting with those is fine but we will need to review them again when adding them to the nova repo for correctness.\n\nFYI API sample tests do this for us. At least for APIs that have not been removed.\n\n\u003e i agree that doing this for remvoed api like nova network is not somethign we should do. deprecated apis woudl be a low priority but dependign on the continued usage there may be some value so as long a we have not moved the api but just deprecated some usage of the api i.e. the force parmaters for live migration we still proably would want validatidtion for the deprecated microverions.\n\nWe\u0027ll actually need to do this if we want to use the schema for our api-ref docs.\n\n\u003e that would be a lower priorty for me then covering the actully supproted parts.\n\u003e so i would conder validateion fo deprectated functionality in older microversion to be a strech goal.\n\nAgreed.",
|
|
"parentUuid": "b7f7acf1_c21a6237",
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "871724db_705d5722",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 112,
|
|
"author": {
|
|
"id": 11604
|
|
},
|
|
"writtenOn": "2024-03-27T12:20:36Z",
|
|
"side": 1,
|
|
"message": "right but unless we make this an entirly optional feature we muct make nova not have a hard dep on those tools\n\nwhile i am warming to rust its not an approved langruge for use in openstack.\nso the tools artem ahs created cannot be a hard depency of nova.\nhttps://github.com/openstack/governance/blob/master/resolutions/20150901-programming-languages.rst\n\nwe are allowed to use other languages for experiment but we cant create new openstack porjects in rust today.\n\nfrom my perspective there is a gray line around using somethign like py03\npyoxide will allow rust tool to be exposed via python bindings so the usage of the libs would still be via python and there is no diffent to having a c module or rust module in my view.\n\nwhen talking about clis there may be addtional leyway there that would not be affored if its driectly called by nova runtime code.\nthe main implication for that is we will need to keep the api ref/schema generation\nopetional like the pdf generations.\n\ni.e. if you have the tool installsed then a given tox target can be used to generate the openapi schemea docs \n\nif you dont then the normal docs target shoudl be able to generate the api ref as normal.\n\nif we want to change that we need to add rust just like go was added \n\nhttps://github.com/openstack/governance/blob/master/resolutions/20170329-golang-use-case.rst\n\ni think we should do that for what its worth.",
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "7aca9bbd_6983dd7a",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 112,
|
|
"author": {
|
|
"id": 15334
|
|
},
|
|
"writtenOn": "2024-03-27T17:13:52Z",
|
|
"side": 1,
|
|
"message": "\u003e right but unless we make this an entirly optional feature we muct make nova not have a hard dep on those tools\n\u003e \n\u003e while i am warming to rust its not an approved langruge for use in openstack.\n\u003e so the tools artem ahs created cannot be a hard depency of nova.\n\u003e https://github.com/openstack/governance/blob/master/resolutions/20150901-programming-languages.rst\n\u003e \n\u003e we are allowed to use other languages for experiment but we cant create new openstack porjects in rust today.\n\u003e\n\u003e from my perspective there is a gray line around using somethign like py03\n\u003e pyoxide will allow rust tool to be exposed via python bindings so the usage of the libs would still be via python and there is no diffent to having a c module or rust module in my view.\n\u003e \n\u003e when talking about clis there may be addtional leyway there that would not be affored if its driectly called by nova runtime code.\n\u003e the main implication for that is we will need to keep the api ref/schema generation\n\u003e opetional like the pdf generations.\n\u003e \n\u003e i.e. if you have the tool installsed then a given tox target can be used to generate the openapi schemea docs \n\u003e \n\u003e if you dont then the normal docs target shoudl be able to generate the api ref as normal.\n\u003e \n\u003e if we want to change that we need to add rust just like go was added \n\u003e \n\u003e https://github.com/openstack/governance/blob/master/resolutions/20170329-golang-use-case.rst\n\u003e \n\u003e i think we should do that for what its worth.\n\ngtema\u0027s tools are all Python based. It has to be, since we\u0027re introspecting stuff. The Rust code he has is to generate a CLI which is not directly related to this effort.",
|
|
"parentUuid": "871724db_705d5722",
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "512c6185_9ba2c30e",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 126,
|
|
"author": {
|
|
"id": 11604
|
|
},
|
|
"writtenOn": "2024-03-27T12:20:36Z",
|
|
"side": 1,
|
|
"message": "+1\n\ni don\u0027t like the idea of any official schema being released outside of the project that maintains the API.\n\nim entrily fine with enhancing nova to provide this sepecially if we can take a code as schma approch where we can genrate it form annotation on the code.\nor other in tree data.",
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "c03f981a_82c31d32",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 126,
|
|
"author": {
|
|
"id": 15334
|
|
},
|
|
"writtenOn": "2024-03-28T10:29:26Z",
|
|
"side": 1,
|
|
"message": "Acknowledged",
|
|
"parentUuid": "512c6185_9ba2c30e",
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "0e57b3aa_aa76b158",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 139,
|
|
"author": {
|
|
"id": 11604
|
|
},
|
|
"writtenOn": "2024-03-27T12:20:36Z",
|
|
"side": 1,
|
|
"message": "this might be a good way in functional or tempest test ot catch if a patch modifed an api without updating the schemea. but yes i think rusing this in production woudl be wrong.\n\nthe operatiosn would still ahve been performed and if something was added to the server show that was not allowed by the schema then you would not be able to discover why and it would casclade and break list\n\nso i think the error mode is probably better modled as a workaround or something similar to make it clear this should never be used in production.\n\ni.e.\n[workarounds]\nresponce_validation_is_fatal\u003dTrue|False\n\nto mirror the vif plug is fatal option",
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "161c9bc3_3e88d07e",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 139,
|
|
"author": {
|
|
"id": 15334
|
|
},
|
|
"writtenOn": "2024-03-27T17:13:52Z",
|
|
"side": 1,
|
|
"message": "While I don\u0027t think it should be used in production, I *do* think DevStack should allow setting it and it should be set in integration jobs (or set by default in DevStack) once things are stabilised. Our schemas are our docs so failures should be be somewhat painful IMO.",
|
|
"parentUuid": "0e57b3aa_aa76b158",
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "7c595494_871316b8",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 181,
|
|
"author": {
|
|
"id": 15334
|
|
},
|
|
"writtenOn": "2024-03-26T17:58:37Z",
|
|
"side": 1,
|
|
"message": "required",
|
|
"range": {
|
|
"startLine": 181,
|
|
"startChar": 56,
|
|
"endLine": 181,
|
|
"endChar": 66
|
|
},
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "11e2a5da_7e5deae4",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 181,
|
|
"author": {
|
|
"id": 11604
|
|
},
|
|
"writtenOn": "2024-03-27T12:20:36Z",
|
|
"side": 1,
|
|
"message": "i was going to suggest \"required by tests\" as in could we write some generic test that asserts there is at least one schema for ever microverion.\n\nwhere that would bereak is the removeal fo an api. but that realy happens.\nin either case it would be nice is we had something liek the opbject hash tests tha t will fail if you moify an object that could fail if you modify an api\n\nthat might not be practical however.",
|
|
"parentUuid": "7c595494_871316b8",
|
|
"range": {
|
|
"startLine": 181,
|
|
"startChar": 56,
|
|
"endLine": 181,
|
|
"endChar": 66
|
|
},
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "c11bf33f_ab80a0d9",
|
|
"filename": "specs/2024.2/approved/openapi.rst",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 181,
|
|
"author": {
|
|
"id": 15334
|
|
},
|
|
"writtenOn": "2024-03-27T17:13:52Z",
|
|
"side": 1,
|
|
"message": "Acknowledged",
|
|
"parentUuid": "11e2a5da_7e5deae4",
|
|
"range": {
|
|
"startLine": 181,
|
|
"startChar": 56,
|
|
"endLine": 181,
|
|
"endChar": 66
|
|
},
|
|
"revId": "506e904394ef20ad7729a38c9366aa5406d68568",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
}
|
|
]
|
|
} |