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

Bodhi updates on Greenwave message #3200

Merged
merged 1 commit into from Jun 7, 2019

Conversation

5 participants
@tdawson
Copy link
Collaborator

commented Apr 30, 2019

Bodhi updates updates_test_gating_status when Greenwave sends a message saying it's ready.

@tdawson

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 30, 2019

I belive the code that reacts to the code is working, but my test is not.
For my test, I am grabbing the update associated with my message, and manually changing the test_gating_status. This should (and does) change test_gating_passed to False. I then run a message through the function and it attempts to change it back. I believe it doesn't, but only because I manually changed it in the first place.
When I do these changes manually, I don't get a successful change from normal, to false, and back to normal.
This is why this is still in Draft format while I figure out a correct way to do the test.

@bowlofeggs

This comment has been minimized.

Copy link
Member

commented May 2, 2019

Make sure you sign the DCO in your commit messages, as described here:

https://bodhi.fedoraproject.org/docs/developer/index.html#contribution-guidelines

This is why the DCO check has failed.

Show resolved Hide resolved bodhi/server/consumers/greenwave.py Outdated
Show resolved Hide resolved bodhi/server/consumers/greenwave.py Outdated
Show resolved Hide resolved bodhi/server/consumers/greenwave.py Outdated
Show resolved Hide resolved bodhi/server/consumers/greenwave.py
Show resolved Hide resolved bodhi/tests/server/consumers/test_greenwave.py Outdated
Show resolved Hide resolved bodhi/tests/server/consumers/test_greenwave.py
Show resolved Hide resolved bodhi/tests/server/consumers/test_greenwave.py Outdated

@tdawson tdawson force-pushed the tdawson:greenwave branch from 63efc3f to 943c172 May 2, 2019

@tdawson tdawson marked this pull request as ready for review May 2, 2019

@tdawson tdawson requested a review from fedora-infra/bodhi as a code owner May 2, 2019

@tdawson tdawson force-pushed the tdawson:greenwave branch 2 times, most recently from 4cf7f09 to a4efa3c May 2, 2019

@tdawson tdawson force-pushed the tdawson:greenwave branch 3 times, most recently from d412bca to b0c6a9c May 6, 2019

Show resolved Hide resolved bodhi/server/consumers/greenwave.py Outdated
Show resolved Hide resolved bodhi/server/consumers/greenwave.py Outdated

@cverna cverna added this to To do in CI Gating via automation May 9, 2019

@cverna cverna moved this from To do to Need Review in CI Gating May 9, 2019

@cverna
Copy link
Member

left a comment

I think we need to remove the mock on @mock.patch('bodhi.server.consumers.greenwave.Update.update_test_gating_status') for the test to pass.

Would be good to also remove the debug print statements 👍

Show resolved Hide resolved bodhi/tests/server/consumers/test_consumers.py Outdated
Show resolved Hide resolved bodhi/tests/server/consumers/test_greenwave.py Outdated
@tdawson

This comment has been minimized.

Copy link
Collaborator Author

commented May 15, 2019

My comment on this pull request was hidden in the commit.
"Added several print statements to view what values are as tests go along.
I can see that the update is properly being processed, but I can not get the results back to the test."

This commit was not intended to be ready, in fact it fails the tests. I pushed it up so that others could easily look at it and/or try it in vagrant.

All random print statements, and odd commented out lines will be removed. Right now it's a work in progress.

@tdawson

This comment has been minimized.

Copy link
Collaborator Author

commented May 16, 2019

Although the code in greenwave.py is working, the tests are not. The central problem is that the tests don't seem to be able to grab the Update that greenwave.py is working on.
This last commit has comments on what I think is important parts of the code, hopefully allowing someone else to jump in and see the progress and hopefully be able to fix it.

@cverna

This comment has been minimized.

Copy link
Member

commented May 17, 2019

I think we will need to rebase this PR once #3232 is merged since it refactor how we call the consumers. Let's wait for that then we can see how to get the tests to pass :-)

@cverna cverna force-pushed the tdawson:greenwave branch 2 times, most recently from 0e7e6f3 to b42d8ba May 23, 2019

@tdawson tdawson force-pushed the tdawson:greenwave branch from 6aa0743 to c0f2fe2 May 23, 2019

@tdawson

This comment has been minimized.

Copy link
Collaborator Author

commented May 23, 2019

Looks very good. Thank you Clement.
I had to make a few minor changes, mostly whitespace, to make btest happy.
I have also squashed the commits, and gave it a good, long descriptive git commit message.
As long as all the tests pass, which I think they will, I think this might be ready.

@tdawson tdawson force-pushed the tdawson:greenwave branch from 5a7a979 to c0f2fe2 May 24, 2019

@bowlofeggs

This comment has been minimized.

Copy link
Member

commented May 29, 2019

@bowlofeggs

This comment has been minimized.

Copy link
Member

commented May 29, 2019

@tdawson tdawson force-pushed the tdawson:greenwave branch 3 times, most recently from 4bfc819 to 7d6d0cb May 29, 2019

@abompard

This comment has been minimized.

Copy link
Member

commented Jun 4, 2019

The current patch looks good to me, so +1, but since I'm not the one who has been reviewing this until now, maybe someone else should +1 it too.

Assert that a greenwave message updates the gating tests status of an update.
"""

self.handler.db_factory = TransactionalSessionMaker(self.Session)

This comment has been minimized.

Copy link
@nphilipp

nphilipp Jun 4, 2019

Member

Just an idea, this initialization of the test DB session could be moved to TestGreenwaveHandler.setUp() to be available in all test methods that might use it.

@nphilipp
Copy link
Member

left a comment

Looks good to me!

@cverna cverna requested review from cverna and bowlofeggs Jun 4, 2019

@cverna

cverna approved these changes Jun 4, 2019

Copy link
Member

left a comment

I wrote this PR with @tdawson, so I approve these changes 😛

@cverna cverna removed the request for review from bowlofeggs Jun 5, 2019

Haven't looked at it in a while and three other people have.

@cverna cverna force-pushed the tdawson:greenwave branch 3 times, most recently from ea65651 to ed7d280 Jun 6, 2019

Add Greenwave message consumer
The geenwave message consumer listens for messages from
  greenwave.  When it receives one, it determines what
  update it relates to, and updates the
  test_gating_status for that update.
Various tests has been added also to not only check
  that the code works, but that it can handle a
  variety of bad messages and errors.
Signed-off-by: Troy Dawson <tdawson@redhat.com>

@cverna cverna force-pushed the tdawson:greenwave branch 3 times, most recently from a012f67 to 564f73c Jun 7, 2019

@mergify mergify bot merged commit fde8eef into fedora-infra:develop Jun 7, 2019

28 of 30 checks passed

continuous-integration/jenkins/pr-head This commit is being built
Details
rawhide-integration running
Details
DCO DCO
Details
Summary 2 rules match and 4 potential rules
Details
f29-build
Details
f29-diff-cover
Details
f29-docs
Details
f29-integration
Details
f29-integration-build
Details
f29-unit
Details
f30-build
Details
f30-diff-cover
Details
f30-docs
Details
f30-integration
Details
f30-integration-build
Details
f30-unit
Details
pip-build
Details
pip-diff-cover
Details
pip-docs
Details
pip-flake8
Details
pip-integration
Details
pip-integration-build
Details
pip-mypy
Details
pip-pydocstyle
Details
pip-unit
Details
rawhide-build
Details
rawhide-diff-cover
Details
rawhide-docs
Details
rawhide-integration-build
Details
rawhide-unit
Details

CI Gating automation moved this from Need Review to Merged to develop Jun 7, 2019


build = Build.get(subject_identifier)
if build is None:
raise BodhiException(f"Couldn't find build {subject_identifier} in DB")

This comment has been minimized.

Copy link
@bowlofeggs

bowlofeggs Jun 7, 2019

Member

Raising exceptions here and elsewhere will cause a severe problem: #3302

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.