Getting Started: Authenticate Users

[bshaffer]

See https://cloud.google.com/python/getting-started/authenticate-users

[grayside]

Looks good, from a couple of clues, I've inferred the auth involved here is verifying requests that come in via IAP. Standalone it seems like it may be worth explicitly calling that out, maybe a README?

[grayside]

Intended?

[bshaffer]

I am on the fence because I don't want to be too strict on the requirements here, since it's a sample app. If it's strict to one set of packages, it could break on the next version (e.g. composer.lock is for PHP 7.2 and so it fails on PHP 7.3). However, if we omit composer.lock, the opposite could happen (e.g. a newer, incompatible version of a package is installed and breaks).

But if semantic versioning is observed, I think the second is not very likely, so I'd prefer to leave it out. What do you think?

[dwsupplee]

Why not use lumen like the bookshelf example? It would be nice to wrap some of the functions in something that can contain a bit of state, so you don't need to create multiple guzzle clients per get_metadata request and can cache some of the static bits of information.

[bshaffer]

@dwsupplee I think Lumen would be a bit much for this sample, since it'd add several additional files and this is essentially a single endpoint. Although I do agree the switch statement is not my favorite thing in the world, I still think the "microframeworks" for PHP out there are too big for examples like this.

[bshaffer]

@dwsupplee Thanks again for your super thorough review! Have I addressed all your suggestions to an acceptable extent?

[dwsupplee]

Happy to help 😄. Just one last minor item - #207 (comment) (I think it got buried since the original comment was already addressed).

[bshaffer]

@dwsupplee good catch!!