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

healthcare.api-client.dicom.dicom_stores_test: test_patch_dicom_store failed #3756

Closed
flaky-bot bot opened this issue May 13, 2020 · 8 comments · Fixed by #3758
Closed

healthcare.api-client.dicom.dicom_stores_test: test_patch_dicom_store failed #3756

flaky-bot bot opened this issue May 13, 2020 · 8 comments · Fixed by #3758
Assignees
Labels
api: healthcare Issues related to the Cloud Healthcare API API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@flaky-bot
Copy link

flaky-bot bot commented May 13, 2020

This test failed!

To configure my behavior, see the Build Cop Bot documentation.

If I'm commenting on this issue too often, add the buildcop: quiet label and
I will stop commenting.


commit: 41d7204
buildURL: Build Status, Sponge
status: failed

Test output
Traceback (most recent call last):
  File "/tmpfs/src/github/python-docs-samples/healthcare/api-client/dicom/dicom_stores_test.py", line 123, in test_patch_dicom_store
    assert 'Patched DICOM store' in out
AssertionError: assert 'Patched DICOM store' in 'Created DICOM store: test_dicom_store_1589382493\nError, DICOM store not patched: \nDeleted DICOM store: test_dicom_store_1589382493\n'
@flaky-bot flaky-bot bot added buildcop: issue priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels May 13, 2020
@gguuss gguuss added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. labels May 13, 2020
@gguuss
Copy link
Contributor

gguuss commented May 13, 2020

Reassigning to Justin, I'm not sure the dicom samples have a proper owner unless it's still Benedict.

@noerog
Copy link
Contributor

noerog commented May 13, 2020

Yeah this is caused by a missing permission on the Cloud Healthcare Service Agent service account in IAM.

I added Pub/Sub Publisher to the service account in the test project and will see if that fixes it.

@JustinBeckwith JustinBeckwith added the api: healthcare Issues related to the Cloud Healthcare API API. label May 13, 2020
@JustinBeckwith
Copy link
Contributor

@gguuss from a process perspective, whenever you add the appropriate api: <boop> label, it will do the right attribution in sloth to go/yoshi-live would know the OnRamp team is on the hook for it. @tbpg is exploring the idea of blunderbuss style assignment based on API, but it's not there quite yet :) Co-assigned this to @tmatsuo since Python is his area!

@gguuss
Copy link
Contributor

gguuss commented May 13, 2020

@gguuss from a process perspective, whenever you add the appropriate api: <boop> label, it will do the right attribution in sloth to go/yoshi-live would know the OnRamp team is on the hook for it. @tbpg is exploring the idea of blunderbuss style assignment based on API, but it's not there quite yet :) Co-assigned this to @tmatsuo since Python is his area!

SGTM, thanks for the pointer

@tmatsuo
Copy link
Contributor

tmatsuo commented May 13, 2020

I guess our test service account in the test project is the Cloud Healthcare Service Agent. I will add the Pub/Sub publisher role to the service account and run the test locally.

@tmatsuo
Copy link
Contributor

tmatsuo commented May 13, 2020

It doesn't. I looked at the test code. It doesn't seem that the test is creating the topic in question.
I'll change it to create the topic and see how it goes.

@tmatsuo
Copy link
Contributor

tmatsuo commented May 13, 2020

The test started to pass with the above change. I don't know why it used to pass. Maybe the server side changed?

Anyways, I'll send a PR for this test and also for v1 subdir.

@tmatsuo
Copy link
Contributor

tmatsuo commented May 13, 2020

My guess is that, the server changed the behavior and now it checks the existence of the Pub/Sub topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: healthcare Issues related to the Cloud Healthcare API API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants