Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't enforce project ID validation when unmarshallig log entries #1808

Merged
merged 2 commits into from
May 23, 2016
Merged

Don't enforce project ID validation when unmarshallig log entries #1808

merged 2 commits into from
May 23, 2016

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented May 18, 2016

The back-end apparently returns numeric project IDs in some cases.

Update gcloud._helpers._name_from_project_path to allow passing its project argument as None to disable the validation.

Closes: #1806.

The back-end apparently returns numeric project IDs in some cases.

Closes: #1806.
@tseaver tseaver added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. api: logging Issues related to the Cloud Logging API. labels May 18, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 18, 2016
@dhermes
Copy link
Contributor

dhermes commented May 18, 2016

The decision to fully disable the check instead of to make it optional was due to the deep nesting of method calls? Could we put a flag on Client to disable "globally"?

/cc @waprin

@tseaver
Copy link
Contributor Author

tseaver commented May 18, 2016

@dhermes The deep nesting is part of it: the other part is that the validation was a carryover from other places where having the project match up was essential to allow re-use of the existing client, which isn't the case at all for log entries (there are no API calls which originate from them).

If the backend had a "list loggers" API (which it inexplicably doesn't), then we might need to be validating the project when unmarshalling its results.

@tseaver
Copy link
Contributor Author

tseaver commented May 18, 2016

I guess somebody could call client.list_entries() and then try to navigate from one of the returned entries to its logger attribute and use the (potentially invalid) logger to make API calls. Seems a stretch, though.

@dhermes
Copy link
Contributor

dhermes commented May 23, 2016

Sorry for going AWOL. 5 days, eek! LGTM. We can add in more hooks as they come up.

@waprin
Copy link
Contributor

waprin commented May 23, 2016

Btw, returning project numbers been acknowledged as a bug but apparently it won't get fixed for a few months since it will be fixed by some stack migration anyway.

@tseaver tseaver merged commit 59642ce into googleapis:master May 23, 2016
@tseaver tseaver deleted the 1806-logging-relax_project_validation_for_log_entries branch May 23, 2016 17:28
@dhermes dhermes mentioned this pull request Jun 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: logging Issues related to the Cloud Logging API. cla: yes This human has signed the Contributor License Agreement. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants