opensearch-project / opensearch-rs

OpenSearch Rust Client
Apache License 2.0
63 stars 34 forks source link

`cargo make generate-api` : invalid gzip error #190

Closed AbhinavGarg90 closed 1 year ago

AbhinavGarg90 commented 1 year ago

What is the bug?

When cargo make generate-api is run, and you try to download the rest specifications from the default branch, it gives the following error:

:~/osci/opensearch-rs$ cargo make generate-api
[cargo-make] INFO - cargo make 0.37.2
[cargo-make] INFO - Calling cargo metadata to extract project info
[cargo-make] INFO - Cargo metadata done
[cargo-make] INFO - Build File: Makefile.toml
[cargo-make] INFO - Task: generate-api
[cargo-make] INFO - Profile: development
[cargo-make] INFO - Running Task: legacy-migration
[cargo-make] INFO - Execute Command: "cargo" "run" "-p" "api_generator"
    Finished dev [unoptimized + debuginfo] target(s) in 0.07s
     Running `target/debug/run`
Download rest specifications [y/N]: y
Branch to download specification from [default master]: master
Error: invalid gzip header
[cargo-make] ERROR - Error while executing command, exit code: 1
[cargo-make] WARN - Build Failed.

How can one reproduce the bug?

Follow instructions provided in DEVELOPER_GUIDE.md to initalise rust project. run cargo make generate-api when prompted with Download rest specifications [y/N]:, enter y when prompted with Branch to download specification from [default main]:, press ENTER for default

What is the expected behavior?

It should generate the API specs successfully.

What is your host/environment?

WSL Ubuntu 22.04.3 LTS libssl-dev installed.

Do you have any additional context?

Attempted different branch names, none seemed to work. However, if ENTER is pressed when prompted with Download rest specifications [y/N]: it executes successfully.

samuelorji commented 1 year ago

@AbhinavGarg90 , I don't think the master branch exists, have you tried main ?. I'm testing on my local machine and it seems to give indeterministic defaults, first was 1.x next was main

AbhinavGarg90 commented 1 year ago

@AbhinavGarg90 , I don't think the master branch exists, have you tried main ?. I'm testing on my local machine and it seems to give indeterministic defaults, first was 1.x next was main

I tried different branch names, 1.x, main and master. None worked for me

AbhinavGarg90 commented 1 year ago

Could you suggest where the root of this issue likely is? I am not sure where to start @Xtansia.

Xtansia commented 1 year ago

@AbhinavGarg90 If you look for the download_specs function in api_generator/src/rest_spec/mod.rs, you can check if where it's trying to download from is the right place or not

AbhinavGarg90 commented 1 year ago

I looked a bit into the control flow. I have a few supposed conclusions:

  1. On a fresh repository, we can successfully pass main when prompted for the branch, but this leads to the following error when trying to generate code
    Generate code from rest specifications main [Y/n]: y
    Error: Failed to parse /home/abhinavgarg/osci/opensearch-rs/api_generator/rest_specs/ml.get_trained_models.json because: invalid type: boolean `true`, expected struct Deprecated at line 51 column 26

    I'm guessing there is a certain format that the program expects, and the error causing file does not meet those specifications. Here is a snippet of which line this is ocurring on for the json:

      "include_model_definition":{
        "type":"boolean",
        "required":false,
        "description":"Should the full model definition be included in the results. These definitions can be large. So be cautious when including them. Defaults to false.",
        "default":false,
        "deprecated": true  <---
      },
      "decompress_definition":{
        "type":"boolean",
        "required":false,
        "default":true,
        "description":"Should the model definition be decompressed into valid JSON or returned in a custom compressed format. Defaults to true."
      },
  2. If cargo make generate-api fails, it still erases the rest_specs folder. I am not sure if this is the intended behaviour.
Xtansia commented 1 year ago

Well based off https://github.com/opensearch-project/opensearch-rs/blob/main/api_generator/src/rest_spec/mod.rs#L30 it's not downloading from the right place. But also I believe the copy of rest_specs in this repo have been hand edited so don't exactly match the ones that'd be downloaded.

But also as #192 is to be done at some point, we probably don't want to invest too much effort into using these legacy specs

AbhinavGarg90 commented 1 year ago

so can we close this issue, considering there is another issue open that is aiming to fix the same problem?

Xtansia commented 1 year ago

@AbhinavGarg90 Yes, that's a fair assessment