PR#4662: Some fixes for vagrant environment - pagure

		`@@ -5,9 +5,8 @@`

		`[Service]`
		`Environment="PAGURE_CONFIG=/home/vagrant/pagure.cfg"`
		`- ExecStart=/home/vagrant/.virtualenvs/python3-pagure/bin/python \`
		`- /home/vagrant/devel/pagure-ci/pagure_ci_server.py`
		`+ ExecStart=/home/vagrant/.virtualenvs/python3-pagure/bin/celery worker -A pagure.lib.tasks --loglevel=info -Q pagure_ci`
		`Type=simple`

		`[Install]`
		`- WantedBy=multi-user.target`
		`+ WantedBy=default.target`

dev/ansible/roles/pagure-dev/files/pagure_ev.service

file modified

+1 -1

		`@@ -10,4 +10,4 @@`
		`Type=simple`

		`[Install]`
		`- WantedBy=multi-user.target`
		`+ WantedBy=default.target`

dev/ansible/roles/pagure-dev/files/pagure_webhook.service

file modified

+2 -3

		`@@ -5,9 +5,8 @@`

		`[Service]`
		`Environment="PAGURE_CONFIG=/home/vagrant/pagure.cfg"`
		`- ExecStart=/home/vagrant/.virtualenvs/python3-pagure/bin/python \`
		`- /home/vagrant/devel/pagure-webhook/pagure-webhook-server.py`
		`+ ExecStart=/home/vagrant/.virtualenvs/python3-pagure/bin/celery worker -A pagure.lib.tasks --loglevel=info -Q pagure_webhook`
		`Type=simple`

		`[Install]`
		`- WantedBy=multi-user.target`
		`+ WantedBy=default.target`

dev/ansible/roles/pagure-dev/files/pagure_worker.service

file modified

+1 -1

		`@@ -10,4 +10,4 @@`
		`Restart=on-failure`

		`[Install]`
		`- WantedBy=multi-user.target`
		`+ WantedBy=default.target`

dev/ansible/roles/pagure-dev/tasks/main.yml

file modified

+17 -26

		`@@ -58,12 +58,6 @@`
		`- python3-virtualenv`
		`state: present`

		`- - name: register the libgit2 version installed`
zlopez commented 4 years ago Why this is moved?
		`- shell: rpm -q libgit2\|cut -d \- -f 2\| cut -d \. -f 1,2`
		`- register: libgit2_version`
		`- args:`
		`- warn: false`
		`-`
		`# Add various helpful configuration files`
		`- name: Install a custom bashrc`
		`become_user: "{{ ansible_env.SUDO_USER }}"`
		`@@ -84,6 +78,12 @@`
		`become_user: "{{ ansible_env.SUDO_USER }}"`
		`copy: src=bash_history dest=/home/{{ ansible_env.SUDO_USER }}/.bash_history`

		`+ - name: register the libgit2 version installed`
		`+ shell: rpm -q libgit2\|cut -d \- -f 2\| cut -d \. -f 1,2`
		`+ register: libgit2_version`
		`+ args:`
		`+ warn: false`
		`+`
		`# Install Pagure inside a virtualenv and configure it`
		`- name: Install pygit2 in the virtualenv`
		`become_user: "{{ ansible_env.SUDO_USER }}"`
		`@@ -94,37 +94,27 @@`
		`virtualenv_python: python3`
		`virtualenv_command: virtualenv`

		`- # Install Pagure inside a virtualenv and configure it for python3`
		`- - name: Install pygit2 in the python3 virtualenv`
		`- become_user: "{{ ansible_env.SUDO_USER }}"`
		`- pip:`
		`- name:`
		`- - "pygit2=={{ libgit2_version.stdout_lines[0] }}.*"`
		`- virtualenv: /home/{{ ansible_env.SUDO_USER }}/.virtualenvs/python3-pagure/`
		`- virtualenv_python: python3`
		`-`
		`- name: Install Pagure Python dependencies into a virtualenv`
		`become_user: "{{ ansible_env.SUDO_USER }}"`
		`pip:`
		`requirements: /home/{{ ansible_env.SUDO_USER }}/devel/{{ item }}`
		`virtualenv: /home/{{ ansible_env.SUDO_USER }}/.virtualenvs/python3-pagure/`
		`virtualenv_command: virtualenv`
		`- with_items:`
		`- - "requirements.txt"`
		`- - "requirements-ev.txt"`
		`- - "requirements-testing.txt"`
		`-`
		`- - name: Install Pagure Python dependencies into a python3 virtualenv`
		`- become_user: "{{ ansible_env.SUDO_USER }}"`
		`- pip:`
		`- requirements: /home/{{ ansible_env.SUDO_USER }}/devel/{{ item }}`
		`- virtualenv: /home/{{ ansible_env.SUDO_USER }}/.virtualenvs/python3-pagure/`
		`virtualenv_python: python3`
		`+ virtualenv_site_packages: yes`
		`+ extra_args: --no-use-pep517`
		`with_items:`
		`- "requirements.txt"`
		`- "requirements-ev.txt"`
		`- "requirements-testing.txt"`

		`+ - name: Hotfix python-fedora to make flask_fas_openid compatible with flask >= 1.0`
		`+ become: "{{ ansible_env.SUDO_USER }}"`
		`+ lineinfile:`
		`+ path: /home/{{ ansible_env.SUDO_USER }}/.virtualenvs/python3-pagure/lib/python3.7/site-packages/flask_fas_openid.py`
		`+ regexp: '^ if assume_recent or \(major >'`
		`+ line: ' if assume_recent or (major > 0 or minor >= 10):'`
		`+`
		`- name: Install Pagure package into a virtualenv`
		`become_user: "{{ ansible_env.SUDO_USER }}"`
		`pip:`
		`@@ -206,12 +196,13 @@`
		`- pagure_webhook.service`
		`- pagure_worker.service`

		`- - name: Enable the Pagure systemd services`
		`+ - name: Enable and start the Pagure systemd services`
		`become: False`
		`systemd:`
		`daemon_reload: yes`
		`name: "{{ item }}"`
		`enabled: True`
		`+ state: started`
		`scope: user`
		`with_items:`
		`- pagure`

pagure/hooks/files/git_multimail_upstream.py

file modified

+12 -9

		`@@ -3636,9 +3636,10 @@`
		`"%(objectname) %(objecttype) %(refname)\n"`
		`"%(objectname) %(objecttype) %(refname)"`
		`)`
		`- ref_filter_regex, is_inclusion_filter = (`
		`- self.environment.get_ref_filter_regex()`
		`- )`
		`+ (`
		`+ ref_filter_regex,`
		`+ is_inclusion_filter,`
		`+ ) = self.environment.get_ref_filter_regex()`
		`for line in read_git_lines(`
		`["for-each-ref", "--format=%s" % (fmt,)]`
		`):`
		`@@ -3865,9 +3866,10 @@`

		`def run_as_post_receive_hook(environment, mailer):`
		`environment.check()`
		`- send_filter_regex, send_is_inclusion_filter = environment.get_ref_filter_regex(`
		`- True`
		`- )`
		`+ (`
		`+ send_filter_regex,`
		`+ send_is_inclusion_filter,`
		`+ ) = environment.get_ref_filter_regex(True)`
		`ref_filter_regex, is_inclusion_filter = environment.get_ref_filter_regex(`
		`False`
		`)`
		`@@ -3902,9 +3904,10 @@`
		`environment, mailer, refname, oldrev, newrev, force_send=False`
		`):`
		`environment.check()`
		`- send_filter_regex, send_is_inclusion_filter = environment.get_ref_filter_regex(`
		`- True`
		`- )`
		`+ (`
		`+ send_filter_regex,`
		`+ send_is_inclusion_filter,`
		`+ ) = environment.get_ref_filter_regex(True)`
		`ref_filter_regex, is_inclusion_filter = environment.get_ref_filter_regex(`
		`False`
		`)`

tests/test_pagure_flask.py

file modified

+1 -1

		`@@ -301,7 +301,7 @@`

		`@mock.patch.dict("pagure.config.config", {"EXTERNAL_COMMITTER": config})`
		`def test_is_repo_committer_external_committer_restricting_another_one(`
		`- self`
		`+ self,`
		`):`
		`""" Test is_repo_committer in pagure with EXTERNAL_COMMITTER`
		`configured to give access the provenpackager just for one repo not`

tests/test_pagure_flask_api_issue.py

file modified

+1 -1

		`@@ -467,7 +467,7 @@`
		`issue = copy.deepcopy(FULL_ISSUE_LIST[8])`
		`issue["id"] = 1`
		`self.assertIn(`
		`- "pagure_tests_placebo.png)](/test/issue/raw/files/"`
		`+ "_tests_placebo.png)](/test/issue/raw/files/"`
		`"8a06845923010b27bfd8e7e75acff7badc40d1021b4994e01f5e11ca"`
		`"40bc3abe",`
		`data["issue"]["content"],`

tests/test_tasks.py

file modified

+24 -12

		`@@ -59,9 +59,12 @@`
		`"alice@example.com": MockUser("Alice", "alice@example.com")`
		`}`

		`- num_commits, authors, num_authors, last_time = tasks.commits_author_stats(`
		`- self.repopath`
		`- )`
		`+ (`
		`+ num_commits,`
		`+ authors,`
		`+ num_authors,`
		`+ last_time,`
		`+ ) = tasks.commits_author_stats(self.repopath)`
pingou commented 4 years ago :( I like the old style better

		`self.assertEqual(num_commits, 1)`
		`self.assertEqual(num_authors, 1)`
		`@@ -109,9 +112,12 @@`
		`"alice@example.com": MockUser("Alice", "alice@example.com")`
		`}`

		`- num_commits, authors, num_authors, last_time = tasks.commits_author_stats(`
		`- self.repopath`
		`- )`
		`+ (`
		`+ num_commits,`
		`+ authors,`
		`+ num_authors,`
		`+ last_time,`
		`+ ) = tasks.commits_author_stats(self.repopath)`

		`self.assertEqual(num_commits, 2)`
		`self.assertEqual(num_authors, 1)`
		`@@ -156,9 +162,12 @@`
		`]`
		`self.authors = {}`

		`- num_commits, authors, num_authors, last_time = tasks.commits_author_stats(`
		`- self.repopath`
		`- )`
		`+ (`
		`+ num_commits,`
		`+ authors,`
		`+ num_authors,`
		`+ last_time,`
		`+ ) = tasks.commits_author_stats(self.repopath)`

		`self.assertEqual(num_commits, 1)`
		`self.assertEqual(num_authors, 1)`
		`@@ -206,9 +215,12 @@`
		`]`
		`self.authors = {}`

		`- num_commits, authors, num_authors, last_time = tasks.commits_author_stats(`
		`- self.repopath`
		`- )`
		`+ (`
		`+ num_commits,`
		`+ authors,`
		`+ num_authors,`
		`+ last_time,`
		`+ ) = tasks.commits_author_stats(self.repopath)`

		`self.assertEqual(num_commits, 3)`
		`self.assertEqual(num_authors, 2)`

no initial comment

zlopez commented on line 5 of dev/ansible/roles/pagure-dev/tasks/main.yml 4 years ago

Why this is moved?

jlanda commented 4 years ago

Why this is moved?

Just for readability. now the register task and the task that uses the registered var are one below the other instead of having a bunch of unrelated tasks in the middle, it's easier to read what's going on imo. But I could remove that commit =)

zlopez commented 4 years ago

No need to remove it, if it improves readability.

Otherwise looks good

pingou commented on line 13 of tests/test_tasks.py 4 years ago

:( I like the old style better

pingou commented 4 years ago

Thanks for these fixes!