Go: run goimports as part of artman script

[jeanbza]

Go: run goimports as part of artman script

[andreamlin]

@noahdietz I think that we should just do the goimports feature request for artman if it's feasible. Unless Yoshi's synthtool migrates to using bazel soon, it won't matter if the Bazel support for Go is good to go. AFAIK Yoshi still uses artman for java generation even tho we have Bazel Java support.

[andreamlin]

Can you provide an example PR where versionClient=UNKNOWN is generated? I can't seem to find where this would be set in the gapic-generator codebase, and I'm not sure when artman would introduce another string inside of a generated file.

[vam-google]

This will be fixed in the upcoming Bazel GAPIC generation (Go support is coming soon, probably next week). Bazel generation produces equal to artman output, does not require installation and should be very easy to migrate to from artman. In that case, we can avoid making the enhancement in artman.

[noahdietz]

@jadekler once Go support is available in Bazel generation, shall we opt to move to that rather than make this fix here? Are you ok with that? Or is the friction enough such that we should fix this asap while we migrate?

[jeanbza]

Er... I'm thoroughly confused, sorry :( How do artman, bazel, and micro generators relate to each other? Is bazel just another driver for artman instead of docker containers, or?

[vam-google]

@jadekler For our use case, we can think about this as follows:

• Bazel replaces Docker, but unlike Docker "installing" it means just getting single Bazel binary on the machine (there are also supported by the bazel team docker images with the binary already available, for convenience). This does not need root permissions and does not contaminate the system (i.e. does not require system-wide changes).
• Bazel GAPIC Extensions replace artman, with additional funcitonality (unlike artman it not only generates the code, but also compiles and runs the generated tests, thus validating the generated code plus making it ready for use out of the box in bazel-based projects). Similarly to how grpc stubs can be generated and compiled with <lang>_grpc_library (like go_grpc_library, java_grpc_library` etc) rules, Bazel GAPIC extensions provide _gapic_library rules to generate and compile GAPIC clients.
• Microgenerator is a language-specific generator. Specifically, artman now calls the macro gapic-generator (same binary for all 7 languages) to generate the client. Creating client is more work than simply generating it, another operations are: distributing sources in proper folders, code formatting etc (these are the other things that artman/gapic_bazel_exetnsions do, but the generators don't). Artman does not support microgenerators. Bazel extesnions are supposed to support both old (macro) and new (micro) generators, making future migration to microgenerators easy.

Using bazel extensions means building googleapis project with bazel (running bazel build //path/to/target)

[jeanbza]

Ah, thank you! This is an excellent explanation. How soon is "upcoming", for my edification?

[jeanbza]

It appears as though "upcoming" is several quarters away, from what I've heard in recent meetings. Could this issue be prioritized? I'm not keen to spend several months having to go through manual git diffs of dozens of files. :/

[noahdietz]

@jadekler to be clear, micro-generator support with Bazel is farther out, but I believe Bazel support for the existing GAPIC generator is coming very soon.

[jeanbza]

Ah - thanks!

[jeanbza]

Friendly ping. This has almost been a month. I understand that the bazel stuff is coming soon (tm), but would it at all be possible to prioritize this enhancement now so that we aren't feeling pain however long the transition takes? It really is quite a nuisance every single time we generate clients.

[noahdietz]

The Bazel Go support is being reviewed right now...so close! 🙏

@vam-google can you confirm that the work under review right now will help address this issue? Or is there something we need to add to it to get a fix in with the current PR?

[noahdietz]

sgtm! I'll get on it

[noahdietz]

After some poking around and testing, it looks like usage of goimports isn't the cause of the import naming churn. This can be verified by running goimports -w language/ from within the cloud.google.com/go directory.

I will look at gapic-generator for potential miss naming of the package alias.

[noahdietz]

Found the spot. I'm going to dupe the issue in gapic-generator, and close this one, as the work is not in artman.