Restructure repository #35
Conversation
emily-emily
requested review from
haruska,
jhamon,
ericapywang,
austin-denoble and
ssmith-pc
emily-emily
force-pushed
the
emily/restructure
branch
from
6c684a5 to
c6cc2df
Compare
|
the bash script may be better done as a build.rs file, similar to the build.rs file in our main pinecone-io/pinecone-db/outer-protos/build.rs script. We use tonic-build there for protobufs, but there are a few options depending on what kind of codegen you want to do, and probably the openapi stuff can be done in a similar way. |
They've used tonic-build in This seems fine to me, but let us know if there are any additional ways this could be improved, @spacejam. |
There was a problem hiding this comment.
Overall, I feel like this makes a lot of sense. Having the openapi and protos as modules inside of pinecone_sdk feels a bit easier to reason about, but I suppose we have to do some manual manipulation of the generated code to get it working.
I've left some feedback around some improvements to the actual generation scripts / steps. I think having a variable for the api_version is important, and I've linked to some examples of getting that set up in other repos, but let me know if you have any questions. I had to implement something similar in the Go SDK recently, and can talk through some of the considerations there.
It might be good to get eyes from @haruska, @ssmith-pc, and @jhamon on this if possible, just to make sure the code generation and project-level / Rust-focused aspects make sense here.
Really nice work on this!
| # copy source files from the crate to the module (outdir) | ||
| echo "Copying source files from $tempdir to $outdir" | ||
| mod_header=$'\n#![allow(missing_docs)]\n' | ||
| pushd $PROJECT_DIR |
There was a problem hiding this comment.
Do we need to do this manually because the openapi-generator-cli outputs to a crate rather than a module by default?
I was going to ask why we couldn't put the generated files directly into outdir, but I'm probably missing something about how this actually works. It also seems like this piece of code is slightly altering each generated file to match where they've been moved to. 🤔
I'm not sure if I have strong opinions on modifying the generated code like this, to me it seems fine and a bit analogous to what we do with templates in Python, and if this is consistently done each time we generate the client it should be fine.
I'd be curious if @jhamon had thoughts on this specifically, though.
There was a problem hiding this comment.
Right, there could be better way to do it, but openapi-generator-cli outputs a crate and there are some small modifications that need to be made for the source files to be included as if they were defined as a module (eg. relative import paths). I didn't copy them directly to outdir to map out where I wanted files to go, and it seemed easier to pick out files I wanted rather than figure out all the files to remove. We could clean it up at the end of the script.
Not sure how robust this is moving forward but it seems to work fine right now.
OpenAPI generates Rust code in the form of a crate. To publish our SDK without separately publishing the OpenAPI code as its own crate, we want to include it as a module in our SDK instead of as an external dependency. - Remove workspace and make `pinecone_sdk` the root project. - OpenAPI code generates to temporary folder `.openapi-crate`. - OpenAPI dependencies are added to `Cargo.toml` for `pinecone_sdk`. - Bash script copies source files from the generated crate into `src/openapi` and also updates imports in these files to reflect the new project structure. - Protobuf code is also moved into `src/protos` and turned into a module. - Disables warnings for generated code (openapi and proto). - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update - [x] Infrastructure change (CI configs, etc) - [ ] Non-code change (docs, etc) - [ ] None of the above: (explain here) All existing test cases pass.
Problem
OpenAPI generates Rust code in the form of a crate. To publish our SDK without separately publishing the OpenAPI code as its own crate, we want to include it as a module in our SDK instead of as an external dependency.
Solution
pinecone_sdkthe root project..openapi-crate.Cargo.tomlforpinecone_sdk.src/openapiand also updates imports in these files to reflect the new project structure.src/protosand turned into a module.Type of Change
Test Plan
All existing test cases pass.