-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fix(logging): update test assertion and core version pins #10087
Conversation
Version bumps are needed, as timeouts were introduced to API core.
An empty zone label that was missing is added.
@@ -179,7 +179,8 @@ def test_log_text_with_resource(self): | |||
logger = Config.CLIENT.logger(self._logger_name("log_text_res")) | |||
now = datetime.datetime.utcnow() | |||
resource = Resource( | |||
type="gae_app", labels={"module_id": "default", "version_id": "test"} | |||
type="gae_app", | |||
labels={"module_id": "default", "version_id": "test", "zone": ""}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is something that was already failing on master
, does not seem to be caused by version bumps in this PR.
(I did not research when exactly the failure was introduced, though)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember seeing something about this. It sounds like it changed fairly recently in the backend.
@@ -179,7 +179,8 @@ def test_log_text_with_resource(self): | |||
logger = Config.CLIENT.logger(self._logger_name("log_text_res")) | |||
now = datetime.datetime.utcnow() | |||
resource = Resource( | |||
type="gae_app", labels={"module_id": "default", "version_id": "test"} | |||
type="gae_app", | |||
labels={"module_id": "default", "version_id": "test", "zone": ""}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember seeing something about this. It sounds like it changed fairly recently in the backend.
Towards #10088.
PR checklist