PR#90: Publish using Fedora Messaging schemas - elections

elections

#90 Publish using Fedora Messaging schemas

Merged 3 years ago by bcotton. Opened 3 years ago by abompard.

abompard/elections fedora-messaging-schemas into develop

Run tests with python3 in tox

Aurélien Bompard • 3 years ago

5e816a9

Publish using Fedora Messaging schemas

Aurélien Bompard • 3 years ago

bc84230

fedora_elections/admin.py

file modified

+21 -25

		`@@ -31,6 +31,13 @@`
		`import flask`
		`from sqlalchemy.exc import SQLAlchemyError`
		`from fedora.client import AuthError`
		`+ from fedora_elections_messages import (`
		`+ NewElectionV1,`
		`+ EditElectionV1,`
		`+ NewCandidateV1,`
		`+ EditCandidateV1,`
		`+ DeleteCandidateV1,`
		`+ )`

		`from fedora_elections import fedmsgshim`
		`from fedora_elections import forms`
		`@@ -109,13 +116,11 @@`

		`SESSION.commit()`

		`- fedmsgshim.publish(`
		`- topic="election.new",`
		`- msg=dict(`
		`+ fedmsgshim.publish(NewElectionV1(body=dict(`
		`agent=flask.g.fas_user.username,`
		`election=election.to_json(),`
		`- )`
		`- )`
		`+ )`
		`+ ))`

		`flask.flash('Election "%s" added' % election.alias)`
		`return flask.redirect(flask.url_for(`
		`@@ -197,13 +202,11 @@`
		`SESSION.delete(admingrp)`

		`SESSION.commit()`
		`- fedmsgshim.publish(`
		`- topic="election.edit",`
		`- msg=dict(`
		`+ fedmsgshim.publish(EditElectionV1(body=dict(`
		`agent=flask.g.fas_user.username,`
		`election=election.to_json(),`
		`)`
		`- )`
		`+ ))`
		`flask.flash('Election "%s" saved' % election.alias)`
		`return flask.redirect(flask.url_for(`
		`'admin_view_election', election_alias=election.alias))`
		`@@ -257,14 +260,12 @@`
		`SESSION.add(candidate)`
		`SESSION.commit()`
		`flask.flash('Candidate "%s" saved' % candidate.name)`
		`- fedmsgshim.publish(`
		`- topic="candidate.new",`
		`- msg=dict(`
		`+ fedmsgshim.publish(NewCandidateV1(body=dict(`
		`agent=flask.g.fas_user.username,`
		`election=candidate.election.to_json(),`
		`candidate=candidate.to_json(),`
		`)`
		`- )`
		`+ ))`
		`return flask.redirect(flask.url_for(`
		`'admin_view_election', election_alias=election.alias))`

		`@@ -328,14 +329,13 @@`
		`candidates_name.append(cand.name)`
		`else:`
		`flask.flash("There was an issue!")`
		`- fedmsgshim.publish(`
		`- topic="candidate.new",`
		`- msg=dict(`
		`+ continue`
		`+ fedmsgshim.publish(NewCandidateV1(body=dict(`
		`agent=flask.g.fas_user.username,`
		`election=cand.election.to_json(),`
		`candidate=cand.to_json(),`
		`)`
		`- )`
		`+ ))`

		`SESSION.commit()`
		`flask.flash('Added %s candidates' % len(candidates_name))`
		`@@ -386,14 +386,12 @@`

		`SESSION.commit()`
		`flask.flash('Candidate "%s" saved' % candidate.name)`
		`- fedmsgshim.publish(`
		`- topic="candidate.edit",`
		`- msg=dict(`
		`+ fedmsgshim.publish(EditCandidateV1(body=dict(`
		`agent=flask.g.fas_user.username,`
		`election=candidate.election.to_json(),`
		`candidate=candidate.to_json(),`
		`)`
		`- )`
		`+ ))`
		`return flask.redirect(flask.url_for(`
		`'admin_view_election', election_alias=election.alias))`

		`@@ -423,14 +421,12 @@`
		`SESSION.delete(candidate)`
		`SESSION.commit()`
		`flask.flash('Candidate "%s" deleted' % candidate_name)`
		`- fedmsgshim.publish(`
		`- topic="candidate.delete",`
		`- msg=dict(`
		`+ fedmsgshim.publish(DeleteCandidateV1(body=dict(`
		`agent=flask.g.fas_user.username,`
		`election=candidate.election.to_json(),`
		`candidate=candidate.to_json(),`
		`)`
		`- )`
		`+ ))`
		`except SQLAlchemyError as err:`
		`SESSION.rollback()`
		`APP.logger.debug('Could not delete candidate')`

fedora_elections/fedmsgshim.py

file modified

+2 -6

		`@@ -15,13 +15,9 @@`
		`_log = logging.getLogger(__name__)`


		`- def publish(topic, msg): # pragma: no cover`
		`- _log.debug('Publishing a message for %r: %s', topic, msg)`
		`+ def publish(message): # pragma: no cover`
		`+ _log.debug('Publishing a message for %r: %s', message.topic, message.body)`
		`try:`
		`- message = fedora_messaging.api.Message(`
		`- topic='fedora_elections.%s' % topic,`
		`- body=msg`
		`- )`
		`fedora_messaging.api.publish(message)`
		`_log.debug("Sent to fedora_messaging")`
		`except PublishReturned as e:`

fedora_elections/models.py

file modified

		`@@ -267,6 +267,7 @@`
		`''' Return a json representation of this object. '''`
		`return dict(`
		`name=self.name,`
		`+ fas_name=self.fas_name,`
		`url=self.url,`
		`)`

setup.py

file modified

+1 -1

		`@@ -28,7 +28,7 @@`
		`install_requires=[`
		`'Flask', 'SQLAlchemy>=0.7', 'python-fedora', 'kitchen',`
		`'python-openid', 'python-openid-teams', 'python-openid-cla',`
		`- 'Flask-wtf', 'wtforms',`
		`+ 'Flask-wtf', 'wtforms', 'fedora-elections-messages',`
		`],`
		`test_suite="tests",`
		`)`

tests/test_candidate.py

file modified

		`@@ -218,6 +218,7 @@`
		`{`
		`'name': 'Ralph',`
		`'url': 'https://fedoraproject.org/wiki/User:Ralph',`
		`+ 'fas_name': None,`
		`}`
		`)`

		`@@ -227,6 +228,7 @@`
		`{`
		`'name': 'Kevin',`
		`'url': 'https://fedoraproject.org/wiki/User:Kevin',`
		`+ 'fas_name': None,`
		`}`
		`)`

tests/test_flask_admin.py

file modified

+41 -26

		`@@ -33,6 +33,10 @@`

		`import flask`
		`from mock import patch, MagicMock`
		`+ from fedora_messaging.testing import mock_sends`
		`+ from fedora_elections_messages import (`
		`+ NewElectionV1, EditElectionV1, NewCandidateV1, EditCandidateV1, DeleteCandidateV1,`
		`+ )`

		`sys.path.insert(0, os.path.join(os.path.dirname(`
		`os.path.abspath(__file__)), '..'))`
		`@@ -318,8 +322,9 @@`
		`'csrf_token': csrf_token,`
		`}`

		`- output = self.app.post(`
		`- '/admin/new', data=data, follow_redirects=True)`
		`+ with mock_sends(NewElectionV1):`
		`+ output = self.app.post(`
		`+ '/admin/new', data=data, follow_redirects=True)`
		`self.assertEqual(output.status_code, 200)`
		`output_text = output.get_data(as_text=True)`
		`self.assertTrue(`
		`@@ -357,8 +362,9 @@`
		`'csrf_token': csrf_token,`
		`}`

		`- output = self.app.post(`
		`- '/admin/new', data=data, follow_redirects=True)`
		`+ with mock_sends(NewElectionV1):`
		`+ output = self.app.post(`
		`+ '/admin/new', data=data, follow_redirects=True)`
		`self.assertEqual(output.status_code, 200)`
		`output_text = output.get_data(as_text=True)`
		`self.assertTrue(`
		`@@ -502,8 +508,9 @@`
		`'csrf_token': csrf_token,`
		`}`

		`- output = self.app.post(`
		`- '/admin/test_election/', data=data, follow_redirects=True)`
		`+ with mock_sends(EditElectionV1):`
		`+ output = self.app.post(`
		`+ '/admin/test_election/', data=data, follow_redirects=True)`
		`self.assertEqual(output.status_code, 200)`
		`output_text = output.get_data(as_text=True)`
		`self.assertTrue(`
		`@@ -580,8 +587,9 @@`
		`'csrf_token': csrf_token,`
		`}`

		`- output = self.app.post(`
		`- '/admin/test_election2/', data=data, follow_redirects=True)`
		`+ with mock_sends(EditElectionV1):`
		`+ output = self.app.post(`
		`+ '/admin/test_election2/', data=data, follow_redirects=True)`
		`self.assertEqual(output.status_code, 200)`
		`output_text = output.get_data(as_text=True)`
		`self.assertTrue(`
		`@@ -619,8 +627,9 @@`
		`'csrf_token': csrf_token,`
		`}`

		`- output = self.app.post(`
		`- '/admin/test_election2/', data=data, follow_redirects=True)`
		`+ with mock_sends(EditElectionV1):`
		`+ output = self.app.post(`
		`+ '/admin/test_election2/', data=data, follow_redirects=True)`
		`self.assertEqual(output.status_code, 200)`
		`output_text = output.get_data(as_text=True)`
		`self.assertTrue(`
		`@@ -705,8 +714,9 @@`
		`'csrf_token': csrf_token,`
		`}`

		`- output = self.app.post(`
		`- '/admin/test_election3/', data=data, follow_redirects=True)`
		`+ with mock_sends(EditElectionV1):`
		`+ output = self.app.post(`
		`+ '/admin/test_election3/', data=data, follow_redirects=True)`
		`self.assertEqual(output.status_code, 200)`
		`output_text = output.get_data(as_text=True)`
		`self.assertTrue(`
		`@@ -734,8 +744,9 @@`
		`'csrf_token': csrf_token,`
		`}`

		`- output = self.app.post(`
		`- '/admin/test_election3/', data=data, follow_redirects=True)`
		`+ with mock_sends(EditElectionV1):`
		`+ output = self.app.post(`
		`+ '/admin/test_election3/', data=data, follow_redirects=True)`
		`self.assertEqual(output.status_code, 200)`
		`output_text = output.get_data(as_text=True)`
		`self.assertTrue(`
		`@@ -829,9 +840,10 @@`
		`'csrf_token': csrf_token,`
		`}`

		`- output = self.app.post(`
		`- '/admin/test_election/candidates/new', data=data,`
		`- follow_redirects=True)`
		`+ with mock_sends(NewCandidateV1):`
		`+ output = self.app.post(`
		`+ '/admin/test_election/candidates/new', data=data,`
		`+ follow_redirects=True)`
		`self.assertEqual(output.status_code, 200)`
		`output_text = output.get_data(as_text=True)`
		`self.assertTrue(`
		`@@ -919,9 +931,10 @@`
		`'csrf_token': csrf_token,`
		`}`

		`- output = self.app.post(`
		`- '/admin/test_election/candidates/new/multi', data=data,`
		`- follow_redirects=True)`
		`+ with mock_sends(NewCandidateV1, NewCandidateV1, NewCandidateV1):`
		`+ output = self.app.post(`
		`+ '/admin/test_election/candidates/new/multi', data=data,`
		`+ follow_redirects=True)`
		`self.assertEqual(output.status_code, 200)`
		`output_text = output.get_data(as_text=True)`
		`self.assertTrue(`
		`@@ -1029,9 +1042,10 @@`
		`'csrf_token': csrf_token,`
		`}`

		`- output = self.app.post(`
		`- '/admin/test_election/candidates/1/edit', data=data,`
		`- follow_redirects=True)`
		`+ with mock_sends(EditCandidateV1):`
		`+ output = self.app.post(`
		`+ '/admin/test_election/candidates/1/edit', data=data,`
		`+ follow_redirects=True)`
		`self.assertEqual(output.status_code, 200)`
		`output_text = output.get_data(as_text=True)`
		`self.assertTrue(`
		`@@ -1121,9 +1135,10 @@`
		`'csrf_token': csrf_token,`
		`}`

		`- output = self.app.post(`
		`- '/admin/test_election4/candidates/10/delete', data=data,`
		`- follow_redirects=True)`
		`+ with mock_sends(DeleteCandidateV1):`
		`+ output = self.app.post(`
		`+ '/admin/test_election4/candidates/10/delete', data=data,`
		`+ follow_redirects=True)`
		`self.assertEqual(output.status_code, 200)`
		`output_text = output.get_data(as_text=True)`
		`self.assertTrue(`

tox.ini

file modified

+1 -1

		`@@ -1,5 +1,5 @@`
		`[tox]`
		`- envlist = py27,diff-cover`
		`+ envlist = py36,py37,py38,diff-cover`
		`skipsdist = True`

		`[testenv]`

abompard commented 3 years ago

This commit enables publishing Fedora Messaging using the newly written schemas for Elections.

You can find those schemas here, and they are on PyPI too for simpler dependency resolution.

pingou commented 3 years ago

We'll likely have to pip install the new dependency in the Dockerfile used otherwise this is likely not going to work when we push it to openshift.

That being said the PR looks good to me.

It may be an idea to give the current elections maintainers access to https://pagure.io/elections-messages/ as well

pingou commented 3 years ago

Checking quickly the schemas, the url field of Candidates is nullable, so https://pagure.io/elections-messages/blob/dev/f/fedora_elections_messages/base.py#_54 likely needs to be adjusted (and the tests as well)

rebased onto ba48e17

3 years ago

1 new commit added

Run tests with python3 in tox

3 years ago

abompard commented 3 years ago

Checking quickly the schemas, the url field of Candidates is nullable, so https://pagure.io/elections-messages/blob/dev/f/fedora_elections_messages/base.py#_54 likely needs to be adjusted (and the tests as well)

Oh good catch, it's not absent, it's None. Fixed the schemas.

We'll likely have to pip install the new dependency in the Dockerfile used otherwise this is likely not going to work when we push it to openshift.

Possibly. That would be in Ansible, right?

It may be an idea to give the current elections maintainers access to https://pagure.io/elections-messages/ as well

Will do.

pingou commented 3 years ago

We'll likely have to pip install the new dependency in the Dockerfile used otherwise this is likely not going to work when we push it to openshift.

Possibly. That would be in Ansible, right?

Actually, we're using s2i here, so it may get things from the requirements.txt
already.

rebased onto bc84230

3 years ago

asaleh commented 3 years ago

Ok, I reviewed the code, re-run all of the tests and even succesfuly built the project locally with s2i (that should mean there will be no problems when building in openshift)