json spec: allow null for documentation url #55749

jakelandis · 2020-04-24T20:19:19Z

This commit allows the JSON schema's documentation.url property to have a null value.
This can useful for cases where a feature is under development, and does not have
documentation published yet.

russcam

I left some comments

russcam · 2020-04-29T08:51:58Z

rest-api-spec/src/main/resources/schema.json

@@ -130,7 +130,7 @@
            "additionalProperties": false,
            "properties": {
                "url": {
-                    "type": "string",
+                    "type": ["string", "null"],


Can the use of null be constrained only to APIs that have stability beta or experimental? This would align with APIs that are under development, whilst still providing benefit of having links for stable APIs

I made the null conditional. So that only stable requires a string, the others (beta, experimental, and private) now allow null. 0b37977

As you pointed out below, this does indeed cause a validation error on the two ml specs below we will need to sort out first.

russcam · 2020-04-29T08:54:55Z

x-pack/plugin/build.gradle

@@ -119,6 +119,4 @@ validateRestSpec {
  ignore 'autoscaling.delete_autoscaling_policy.json'
  ignore 'autoscaling.get_autoscaling_policy.json'
  ignore 'autoscaling.put_autoscaling_policy.json'
-  ignore 'ml.validate.json'


I think these are stable APIs, so would clash with the previous comment about allowing null only for beta and experimental APIs. Perhaps we can link to ML jobs documentation for these specs for now? For example,

https://www.elastic.co/guide/en/machine-learning/current/ml-jobs.html

jakelandis · 2020-04-29T20:01:21Z

@elastic/machine-learning - Is there any public documentation for /_ml/anomaly_detectors/_validate or /_ml/anomaly_detectors/_validate/detector ? Given the options of "stable", "beta", "experimental", "private" which you choose to describe these APIs ?

droberts195 · 2020-05-07T16:23:04Z

Is there any public documentation for /_ml/anomaly_detectors/_validate or /_ml/anomaly_detectors/_validate/detector ?

No. There's no HLRC either. If I remember correctly the thinking was that they weren't as useful as the job validation that the ML UI does, so we didn't want to encourage people to use them. Officially they don't exist.

Given the options of "stable", "beta", "experimental", "private" which you choose to describe these APIs ?

Given the above I guess they have to be "private". Will that mean none of the language clients include them? That would match the lack of HLRC.

If having spec files is causing too many problems we could just delete the spec files altogether.

jakelandis · 2020-05-07T18:07:06Z

thanks @droberts195, I changed the stability to 'private' and the validation should pass now with the conditional to allow null documentation.

@russcam - not sure about the passivity for changing this to private. Any concerns from the client side ? Also, you may want to spot check that the conditional works correctly with what ever the clients are using to validate the spec. I am not 100% sure that merging of definitions from two places is fully part of the schema validation spec.

jakelandis · 2020-05-07T18:16:25Z

@elasticmachine update branch

jakelandis · 2020-05-12T00:52:28Z

@elasticmachine update branch

jakelandis · 2020-05-12T02:07:50Z

Test failures are related: java.lang.IllegalArgumentException: No enum constant org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi.Stability.PRIVATE It looks like we don't support private stability in the test runner. I will fix with this PR (tomorrow).

russcam · 2020-05-12T11:37:52Z

Test failures are related: java.lang.IllegalArgumentException: No enum constant org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi.Stability.PRIVATE It looks like we don't support private stability in the test runner. I will fix with this PR (tomorrow

My apologies @jakelandis; the "private" stability was discussed but not implemented in the end, so the schema needs to be updated to remove it as an option. #56104 is looking to introduce a "visibility" property that had a notion of a "private" API.

russcam

I've left some suggestions that would allow us to move forward with this.

The allowed types for Documentation based on stability looks good.

x-pack/plugin/src/test/resources/rest-api-spec/api/ml.validate.json

x-pack/plugin/src/test/resources/rest-api-spec/api/ml.validate_detector.json

x-pack/plugin/src/test/resources/rest-api-spec/api/ml.validate.json

x-pack/plugin/src/test/resources/rest-api-spec/api/ml.validate_detector.json

…_detector.json Co-authored-by: Russ Cam <[email protected]>

….json Co-authored-by: Russ Cam <[email protected]>

…_detector.json Co-authored-by: Russ Cam <[email protected]>

jakelandis · 2020-05-12T13:03:39Z

Thanks @russcam - I have applied the suggestions.

russcam

LGTM 👍

This commit allows the JSON schema's documentation.url property to have a null value. This can useful for cases where a feature is under development, and does not have documentation published yet. This commit also adds a documentation.url for two ml resources.

json spec: allow null for documentation url

25dcd29

jakelandis added >non-issue v8.0.0 v7.8.0 v7.7.1 Team:Clients Meta label for clients team labels Apr 24, 2020

jakelandis requested a review from russcam April 24, 2020 20:19

russcam reviewed Apr 29, 2020

View reviewed changes

jakelandis added 2 commits April 29, 2020 16:44

conditional null

0b37977

Merge remote-tracking branch 'upstream/master' into null_doc_url

dddf74b

pugnascotia added v7.8.1 v7.9.0 and removed v7.8.0 v7.8.1 labels May 6, 2020

move ml specs to private

f3e5af7

Merge branch 'master' into null_doc_url

ccbf2dc

Merge branch 'master' into null_doc_url

3ca7d6f

russcam suggested changes May 12, 2020

View reviewed changes

jakelandis and others added 4 commits May 12, 2020 08:00

Update x-pack/plugin/src/test/resources/rest-api-spec/api/ml.validate…

0e606db

…_detector.json Co-authored-by: Russ Cam <[email protected]>

Update x-pack/plugin/src/test/resources/rest-api-spec/api/ml.validate…

5e47c78

….json Co-authored-by: Russ Cam <[email protected]>

Update x-pack/plugin/src/test/resources/rest-api-spec/api/ml.validate…

e3d7004

….json Co-authored-by: Russ Cam <[email protected]>

Update x-pack/plugin/src/test/resources/rest-api-spec/api/ml.validate…

93dfadf

…_detector.json Co-authored-by: Russ Cam <[email protected]>

russcam approved these changes May 12, 2020

View reviewed changes

jakelandis merged commit 525522e into elastic:master May 12, 2020

jakelandis deleted the null_doc_url branch May 12, 2020 17:51

jakelandis added backport pending v7.8.1 labels May 12, 2020

jakelandis mentioned this pull request May 12, 2020

[7.7] json spec: allow null for documentation url (#55749) #56623

Merged

jakelandis mentioned this pull request May 12, 2020

[7.8] json spec: allow null for documentation url (#55749) #56624

Merged

jakelandis mentioned this pull request May 12, 2020

[7.x] json spec: allow null for documentation url (#55749) #56625

Merged

jakelandis removed the backport pending label May 12, 2020

russcam mentioned this pull request Jul 17, 2020

Documentation urls for ml.validate, ml.validate_detector #54253

Closed

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

json spec: allow null for documentation url #55749

json spec: allow null for documentation url #55749

jakelandis commented Apr 24, 2020

russcam left a comment

russcam Apr 29, 2020

jakelandis Apr 29, 2020 •

edited

Loading

russcam Apr 29, 2020

jakelandis commented Apr 29, 2020

droberts195 commented May 7, 2020

jakelandis commented May 7, 2020

jakelandis commented May 7, 2020

jakelandis commented May 12, 2020

jakelandis commented May 12, 2020

russcam commented May 12, 2020

russcam left a comment

jakelandis commented May 12, 2020

russcam left a comment

json spec: allow null for documentation url #55749

json spec: allow null for documentation url #55749

Conversation

jakelandis commented Apr 24, 2020

russcam left a comment

Choose a reason for hiding this comment

russcam Apr 29, 2020

Choose a reason for hiding this comment

jakelandis Apr 29, 2020 • edited Loading

Choose a reason for hiding this comment

russcam Apr 29, 2020

Choose a reason for hiding this comment

jakelandis commented Apr 29, 2020

droberts195 commented May 7, 2020

jakelandis commented May 7, 2020

jakelandis commented May 7, 2020

jakelandis commented May 12, 2020

jakelandis commented May 12, 2020

russcam commented May 12, 2020

russcam left a comment

Choose a reason for hiding this comment

jakelandis commented May 12, 2020

russcam left a comment

Choose a reason for hiding this comment

jakelandis Apr 29, 2020 •

edited

Loading