BREAKING: Namespace crate items in `api` and `client` modules

[petrosagg]

hey @Byron! I worked a bit on #258 and this is PR is what I came up with. I didn't push the commit that regenerates all the crates for ease of review. Here is the summary:

I put all the generated code under an api module and renamed the cmn module into client as this module is now public. The top level crate re-exports only a handful of items to make it easier to use. Specifically, it exports the main hub type for each API, and the Result, Error, and Delegate items from the client module.

Users that are interested in more items can do so via the dedicated modules.

Lastly, I changed all the doc links to use the new cargo doc linking mechanism that avoids manual generation of html paths.

Let me know what you think!

[Byron]

Thanks so much! I am taking a look right away :).

I didn't push the commit that regenerates all the crates for ease of review.

That's perfect, making the review so much easier. It's much appreciated - after merge I will happily regenerate the APIs and publish new versions to crates.io.

Lastly, I changed all the doc links to use the new cargo doc linking mechanism that avoids manual generation of html paths.

Oooh, I am so excited! I am using that mechanism myself in my recent crates and it's sooo awesome :) (besides being unable to link to trait implementations or their methods, but who wants to be greedy ;)).

[Byron]

Uh, even though I thought I could quickly cargo check everything and prepare a release, it seems that my M1 Macbook Air has difficulties executing the INTEL python binary coming in with the virtual env version used here.

Working around this one isn't as easy as I hoped, so will investigate if a more modern virtualenv version handles different architectures on MacOS already.

Python… what was I thinking 😅

[petrosagg]

Do you know what's going on with the part property? If I understand correctly it has special handling from the code and it is assumed to be a String even though the spec has repeated: true. It doesn't look like something my PR broke but maybe I'm missing something.

[petrosagg]

Hm, looks like this is a change in the schema from google's side. It didn't use to have repeated: true in there. I'll try to adapt the code to use Vecs where appropriate

[petrosagg]

I pushed a commit that makes cargo check pass and for the most part seems fine. The only issue is that it generates incorrect doc tests but I'll wait for your input before digging deeper

[Byron]

Thank you! I will give it another go now and hope that I can get it to run locally.

Regarding part - thanks a lot for digging in already. I must say that I have zero knowledge about the why's of the code anymore besides some high-level knowledge about the structure of the project and the way it's automating everything.

Besides, everything looks good from a glance, and all I would do is run the aforementioned cargo check on everything with make cargo-api ARGS=check and build docs with make docs-all. If these look good (and ignoring the doc tests for now), I would believe it's ready for release.

Please let me know if there are any other questions.

[petrosagg]

Haha, I understand, the process is pretty unwieldy. I wonder how much of the work that mako templates do can be done with rust macros. I'll try make cargo-api ARGS=check now and see how it goes

[Byron]

Thanks a bunch! And the same goes for make cargo-cli ARGS=check to validate the CLI still works with the changed imports I suppose.

Bad news on me trying to get this back to work - it's not just python, it's also Mako which is very outdated and needs python 2, which we all know is now officially end of life. Newer virtualenv versions probably don't support that anymore either.

It's a bit sad as this would mean that I cannot maintain this project anymore… let's try again doing this in INTEL compatibility mode then 😅.

[petrosagg]

@Byron I'm having build errors with make cargo-api ARGS=check even on the current master branch I'm not sure if it's just me or something is broken. Here is part of the output for one of the crates:

error[E0592]: duplicate definitions with name `param`
     --> src/lib.rs:11763:5
      |
11763 | /     pub fn param<T>(mut self, name: T, value: T) -> ProjectInstanceDatabaseOperationListCall<'a, C, A>
11764 | |                                                         where T: AsRef<str> {
      | |___________________________________________________________________________^ duplicate definitions for `param`
...
17730 | /     pub fn param<T>(mut self, name: T, value: T) -> ProjectInstanceDatabaseOperationListCall<'a, C, A>
17731 | |                                                         where T: AsRef<str> {
      | |___________________________________________________________________________- other definition for `param`

error[E0592]: duplicate definitions with name `add_scope`
     --> src/lib.rs:11783:5
      |
11783 | /     pub fn add_scope<T, S>(mut self, scope: T) -> ProjectInstanceDatabaseOperationListCall<'a, C, A>
11784 | |                                                         where T: Into<Option<S>>,
11785 | |                                                               S: AsRef<str> {
      | |___________________________________________________________________________^ duplicate definitions for `add_scope`
...
17750 | /     pub fn add_scope<T, S>(mut self, scope: T) -> ProjectInstanceDatabaseOperationListCall<'a, C, A>
17751 | |                                                         where T: Into<Option<S>>,
17752 | |                                                               S: AsRef<str> {
      | |___________________________________________________________________________- other definition for `add_scope`

error: aborting due to 18 previous errors

Some errors have detailed explanations: E0119, E0428, E0592.
For more information about an error, try `rustc --explain E0119`.
error: could not compile `google-spanner1`

[Byron]

That's new to me, for all I know, maybe there is two codepaths that should conditionally place one or the other (?), but that always run for some reason? It should be easy to manually take a look to see if these definitions are exactly the same or slightly different.

If this issue only exists in one particular API, it might be OK to just blacklist it for now to get to a green state quickly.

[petrosagg]

@Byron right now all the APIs pass cargo check and the CLI has an error that I don't think is caused by this PR

[Byron]

I agree, master unfortunately is already broken, and fixing that certainly shouldn't be on you.

With me still being unable to run this locally despite trying I probably won't be the one fixing this either. All kinds of trouble related to ARM and INTEL being mixed even after successfully getting this old virtualenv to run with the patched python binary it creates.

However, I would be able to publish the crates as it wouldn't necessarily require building them, even though it would very much be like flying blind :/.

The last thing I can imagine doing is using an INTEL mac which I still have access to, but I don't like the implication of eventually not being able to maintain this anymore.

Maybe some day Docker will work on Apple Silicon, so one could try running things in a linux environment.

[Byron]

@petrosagg I am now working on a toolchain update which should allow this PR to be verified and merged. Biting the bullet seemed favourable over letting this project fall out of maintenance.

[Byron]

@petrosagg It looks like I am now able to run the toolchain and build the API. However, building the CLI doesn't work anymore due to ring (a dependency of the outdated hyper) not compiling on Apple Silicon. Newer versions work though.

Switching to the most recent version of hyper is possible as all other dependencies like yup-hyper-mock and yup-oauth2 have already made the switch.

The only thing to fix is the initialization of the CLI as well as some parts of the API.

With your experience, is this something you would want to give a shot? If so, is working in this PR OK for you?

Alternatively I can merge this one and see if I can create a new release with what we have, without rebuilding the CLI docs. Please let me know what's best for you.

[Byron]

After putting in a little more thought here is what I think can be done:

• merge this PR
• update all JSON files
• release a new major version of all APIs only - building doesn't work yet unless dependencies are updated. ring doesn't compile for me anymore.

From there, I can try to get the API to work with the latest hyper/tokio/rust-tls respectively while not exposing the async portion initially if at all possible or feasible. With that, the CLI should build again (even for me) and I can create yet another major release.

Of course I could wait and avoid publishing too many crates, let's see :D.