Skip to content
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

Updated discovery node class - and other ml fixes #3153

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

l-trotta
Copy link
Contributor

@l-trotta l-trotta commented Nov 19, 2024

References for the fixes:

The only thins missing server-side proof is ignore_throttled as query parameter in MlPutJobRequest, I couldn't find it in parent classes either, so I'm trusting the validation on that one.

@pquentin
Copy link
Member

The failure is:

Error: A type cannot end with "Request" or "Response"
At: /home/runner/work/elasticsearch-specification/elasticsearch-specification/elasticsearch-specification/output/typescript/types.ts:46:73

@l-trotta
Copy link
Contributor Author

my bad, messed up an import, fixing it now

This comment was marked as outdated.

@pquentin

This comment was marked as outdated.

This comment has been minimized.

@l-trotta
Copy link
Contributor Author

@pquentin I just understood the source of my confusion: it seems like DiscoveryNode can be written in 2 ways, one is implemented in toXContent and it builds it like this:

          "node": {
              "UdmgtQ3gRly7A3Z6PmLrPQ": {
                  "name": "instance-0000000004",
                  "ephemeral_id": "RjniA_UvRLq0bqo2aYL-jg",
                  "transport_address": "10.45.0.50:19529",
                ...
              }
           }

with the id as an upper level json object, while the method writeTo build it like:

          "node": {
                "name": "instance-0000000004",
                "id": "UdmgtQ3gRly7A3Z6PmLrPQ",
                "ephemeral_id": "RjniA_UvRLq0bqo2aYL-jg",
                "transport_address": "10.45.0.50:19529",
                ...
              }
           }

with the id inside the node object. Both ways seem to be accepted and present in the recordings.
I'm thinking this could be a shortcut, but I'd like to confirm this in the next spec meet

@l-trotta l-trotta marked this pull request as draft November 21, 2024 17:01
@l-trotta l-trotta changed the title Updated discovery node class Updated discovery node class - and other ml fixes Nov 21, 2024

This comment was marked as off-topic.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Copy link
Contributor

Following you can find the validation results for the APIs you have changed.

API Status Request Response
ml.clear_trained_model_deployment_cache 🟢 1/1 1/1
ml.close_job 🟢 64/64 63/63
ml.delete_calendar_event 🟢 4/4 4/4
ml.delete_calendar_job 🟢 3/3 3/3
ml.delete_calendar 🟢 5/5 5/5
ml.delete_data_frame_analytics 🟢 2/2 2/2
ml.delete_datafeed 🟢 3/3 3/3
ml.delete_expired_data 🟢 5/5 5/5
ml.delete_filter 🟢 27/27 27/27
ml.delete_forecast 🟢 3/3 3/3
ml.delete_job 🟢 47/47 47/47
ml.delete_model_snapshot 🟢 2/2 2/2
ml.delete_trained_model_alias 🟢 3/3 3/3
ml.delete_trained_model 🟢 5/5 5/5
ml.estimate_model_memory 🟢 16/16 16/16
ml.evaluate_data_frame 🟢 15/15 15/15
ml.explain_data_frame_analytics 🟢 7/7 7/7
ml.flush_job 🟢 15/15 15/15
ml.forecast 🟢 1/1 1/1
ml.get_buckets 🟢 14/14 14/14
ml.get_calendar_events 🟢 15/15 15/15
ml.get_calendars 🟢 17/17 17/17
ml.get_categories 🟢 12/12 12/12
ml.get_data_frame_analytics_stats 🟢 12/12 12/12
ml.get_data_frame_analytics 🟢 17/17 17/17
ml.get_datafeed_stats 🟢 27/27 27/27
ml.get_datafeeds 🟢 20/20 20/20
ml.get_filters 🟢 13/13 13/13
ml.get_influencers 🟢 11/11 11/11
ml.get_job_stats 🟢 32/32 32/32
ml.get_jobs 🟢 31/31 31/31
ml.get_memory_stats 🟢 6/6 6/6
ml.get_model_snapshot_upgrade_stats 🟢 3/3 3/3
ml.get_model_snapshots 🟢 18/18 18/18
ml.get_overall_buckets 🟢 16/16 15/15
ml.get_records 🟢 8/8 8/8
ml.get_trained_models_stats 🟢 17/17 17/17
ml.get_trained_models 🟢 37/37 37/37
ml.infer_trained_model 🟢 10/10 10/10
ml.info 🟢 10/10 10/10
ml.open_job 🟢 83/83 83/83
ml.post_calendar_events 🟢 12/12 12/12
ml.post_data 🔴 9/11 18/18
ml.preview_data_frame_analytics 🟢 3/3 3/3
ml.preview_datafeed 🟢 17/17 17/17
ml.put_calendar_job 🟢 12/12 12/12
ml.put_calendar 🟢 135/135 135/135
ml.put_data_frame_analytics 🟢 33/33 33/33
ml.put_datafeed 🟢 71/71 71/71
ml.put_filter 🟢 27/27 27/27
ml.put_job 🟢 227/227 225/225
ml.put_trained_model_alias 🟢 13/13 13/13
ml.put_trained_model_definition_part 🟢 1/1 1/1
ml.put_trained_model_vocabulary 🟢 1/1 1/1
ml.put_trained_model 🟢 16/16 16/16
ml.reset_job 🟢 2/2 2/2
ml.revert_model_snapshot 🟢 2/2 2/2
ml.set_upgrade_mode 🟢 6/6 6/6
ml.start_data_frame_analytics 🟢 1/1 1/1
ml.start_datafeed 🟢 24/24 24/24
ml.start_trained_model_deployment 🟢 14/14 14/14
ml.stop_data_frame_analytics 🟢 5/5 5/5
ml.stop_datafeed 🟢 17/17 17/17
ml.stop_trained_model_deployment 🟢 10/10 10/10
ml.update_data_frame_analytics 🟢 2/2 2/2
ml.update_datafeed 🟢 7/7 7/7
ml.update_filter 🟢 3/3 3/3
ml.update_job 🟢 5/5 5/5
ml.update_model_snapshot 🟢 3/3 3/3
ml.update_trained_model_deployment 🟢 4/4 4/4
ml.upgrade_job_snapshot 🟢 3/3 3/3
ml.validate_detector 🟢 2/2 2/2
ml.validate 🟢 3/3 3/3

You can validate these APIs yourself by using the make validate target.

@l-trotta l-trotta marked this pull request as ready for review November 29, 2024 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants