Add primer CI tool 🏴 #1402

cooperlees · 2020-05-10T15:52:27Z

Run in PATH black binary on configured projects
Can set wether we expect changes or not per project
Can set what python versions are supported for a project
if long_checkout True project will not be ran on CI

Will add to CI after I finish unit tests to avoid silly bugs I'm sure I have 🤪

Tests:

Manual Run - Will add unit tests if people think it will be useful
Output:

(b) cooper-mbp1:black cooper$ time /tmp/b/bin/black-primer -k -w /tmp/cooper_primer_1
[2020-05-10 08:48:25,696] INFO: 4 projects to run black over (lib.py:212)
[2020-05-10 08:48:25,697] INFO: Skipping aioexabgp as it's disabled via config (lib.py:166)
[2020-05-10 08:48:25,699] INFO: Skipping bandersnatch as it's disabled via config (lib.py:166)
[2020-05-10 08:48:28,676] INFO: Analyzing results (lib.py:225)
-- primer results 📊 --

2 / 4 succeeded (50.0%) ✅
0 / 4 FAILED (0.0%) 💩
 - 2 projects Disabled by config
 - 0 projects skipped due to Python Version
 - 0 skipped due to long checkout

real	0m3.304s
user	0m9.529s
sys	0m1.019s

ls of /tmp/cooper_primer_1

(b) cooper-mbp1:black cooper$ ls -lh /tmp/cooper_primer_1
total 0
drwxr-xr-x  21 cooper  wheel   672B May 10 08:48 attrs
drwxr-xr-x  14 cooper  wheel   448B May 10 08:48 flake8-bugbear

Addresses #1390

SanketDG

This looks great!

I am assuming this has been already discussed, but can we also rope in some of the top projects here that are mentioned in the README as using Black?

SanketDG · 2020-05-14T14:57:59Z

src/black_primer/lib.py

+LOG = logging.getLogger(__name__)
+
+
+class Results(NamedTuple):


suggestion: implement as a dataclass because we have more reads than writes?

Yeah, not going to do this as benefits are low.

SanketDG · 2020-05-14T14:59:53Z

src/black_primer/lib.py

+    path_parts = repo_url_parts.path[1:].split("/", maxsplit=1)
+
+    repo_path: Path = work_path / path_parts[1].replace(".git", "")
+    cmd = [git_bin, "clone", project_config["git_clone_url"]]


Would doing a shallow clone be performant here? Especially with the long_checkouts option?

Yup - great call.

SanketDG · 2020-05-14T15:25:04Z

src/black_primer/cli.py

+
+
+DEFAULT_CONFIG = Path(__file__).parent / "primer.json"
+DEFAULT_WORKDIR = Path(gettempdir()) / f"primer.{getpid()}"


suggestion: Would a timestamp: primer.timestamp be better here?

Agree. Will change to this.

SanketDG · 2020-05-14T15:28:46Z

src/black_primer/cli.py

+)
+@click.option(
+    "-R",
+    "--rebase",


suggestion: --update or --pull

Is this more common in the Open Source world? I'm stuck thinking in Facebook terms.

I am going to say --rebase is pretty intuitive too, so it's fine either way.

SanketDG · 2020-05-14T15:32:36Z

src/black_primer/lib.py

@@ -0,0 +1,246 @@
+#!/usr/bin/env python3
+


this needs a from __future__ import annotations if used > 3.7

I really need to sit down and understand this more - I'll ping you to understand on IRC

relevant: https://www.python.org/dev/peps/pep-0563

Sorry, why is this needed at all? It's nice to turn the future import on but I'm not sure why it's necessary here.

Without it we get a runtime TypeError:

https://pastebin.com/fve4q60S

I could simplify the typing I guess if we prefer.

SanketDG · 2020-05-14T15:37:53Z

src/black_primer/lib.py

+        _stdout, _stderr = await _gen_check_output(cmd, cwd=repo_path)
+    except asyncio.TimeoutError:
+        results.stats["failed"] += 1
+        LOG.error(f"Running black for {repo_path} timedout ({cmd})")


nit: timed out

SanketDG · 2020-05-14T15:45:22Z

src/black_primer/lib.py

+    config_file: str,
+    work_path: Path,
+    workers: int,
+    keep: bool = False,


question: this is unimplemented for now?

Ahhh good find. This should delete the prokects as we finish with them rather than a large delete at the end. Will add!

cooperlees

Absolutely fantastic review @SanketDG ! Appreciate it. Many good little fixes coming in my next commit due to you.

cooperlees · 2020-05-16T18:12:09Z

src/black_primer/cli.py

+
+
+DEFAULT_CONFIG = Path(__file__).parent / "primer.json"
+DEFAULT_WORKDIR = Path(gettempdir()) / f"primer.{getpid()}"


Agree. Will change to this.

cooperlees · 2020-05-16T18:13:51Z

src/black_primer/cli.py

+)
+@click.option(
+    "-R",
+    "--rebase",


Is this more common in the Open Source world? I'm stuck thinking in Facebook terms.

cooperlees · 2020-05-16T18:42:18Z

src/black_primer/lib.py

+LOG = logging.getLogger(__name__)
+
+
+class Results(NamedTuple):


Yeah, not going to do this as benefits are low.

cooperlees · 2020-05-16T18:42:48Z

src/black_primer/lib.py

@@ -0,0 +1,246 @@
+#!/usr/bin/env python3
+


I really need to sit down and understand this more - I'll ping you to understand on IRC

cooperlees · 2020-05-16T18:55:37Z

src/black_primer/lib.py

+    path_parts = repo_url_parts.path[1:].split("/", maxsplit=1)
+
+    repo_path: Path = work_path / path_parts[1].replace(".git", "")
+    cmd = [git_bin, "clone", project_config["git_clone_url"]]


Yup - great call.

cooperlees · 2020-05-16T18:56:33Z

src/black_primer/lib.py

+    config_file: str,
+    work_path: Path,
+    workers: int,
+    keep: bool = False,


Ahhh good find. This should delete the prokects as we finish with them rather than a large delete at the end. Will add!

- Run in PATH `black` binary on configured projects - Can set wether we expect changes or not per project - Can set what python versions are supported for a project - if `long_checkout` True project will not be ran on CI Will add to CI after I finish unit tests to avoid silly bugs I'm sure I have 🤪 Tests: - Manual Run - Will add unit tests if people think it will be useful - Output: ```shell (b) cooper-mbp1:black cooper$ time /tmp/b/bin/black-primer -k -w /tmp/cooper_primer_1 [2020-05-10 08:48:25,696] INFO: 4 projects to run black over (lib.py:212) [2020-05-10 08:48:25,697] INFO: Skipping aioexabgp as it's disabled via config (lib.py:166) [2020-05-10 08:48:25,699] INFO: Skipping bandersnatch as it's disabled via config (lib.py:166) [2020-05-10 08:48:28,676] INFO: Analyzing results (lib.py:225) -- primer results 📊 -- 2 / 4 succeeded (50.0%) ✅ 0 / 4 FAILED (0.0%) 💩 - 2 projects Disabled by config - 0 projects skipped due to Python Version - 0 skipped due to long checkout real 0m3.304s user 0m9.529s sys 0m1.019s ``` - ls of /tmp/cooper_primer_1 ``` (b) cooper-mbp1:black cooper$ ls -lh /tmp/cooper_primer_1 total 0 drwxr-xr-x 21 cooper wheel 672B May 10 08:48 attrs drwxr-xr-x 14 cooper wheel 448B May 10 08:48 flake8-bugbear ```

- Don't use asyncio.run() ... go back to the past :P - Refactor results into a named tuple of two dicts to avoid typing nightmare - Fix some variable names - Fix bug with rebase logic in git_checkout_or_rebase

cooperlees added the T: enhancement New feature or request label May 10, 2020

cooperlees requested review from ambv, zsol and JelleZijlstra May 10, 2020 15:52

cooperlees force-pushed the black_primer branch from 2cb16f6 to af5aabf Compare May 10, 2020 16:51

cooperlees changed the title ~~Add primer CI tool 💩~~ Add primer CI tool 🏴 May 10, 2020

SanketDG reviewed May 14, 2020

View reviewed changes

cooperlees commented May 16, 2020

View reviewed changes

cooperlees added 5 commits May 16, 2020 13:32

Address mypy 3.6 type errors

a29c80f

- Don't use asyncio.run() ... go back to the past :P - Refactor results into a named tuple of two dicts to avoid typing nightmare - Fix some variable names - Fix bug with rebase logic in git_checkout_or_rebase

Prettier the JSON config file for primer

6a2c786

Delete projects when finished, move dir to be timestamped + shallow copy

cb4bce3

Re-enable disabled projects post @JelleZijlstra's docstring fix

1450318

cooperlees force-pushed the black_primer branch from af80b11 to 1450318 Compare May 16, 2020 20:32

Workaround for future annotations until someone tells me the correct fix

a5b3004

cooperlees merged commit b50a527 into master May 17, 2020

cooperlees deleted the black_primer branch May 17, 2020 00:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add primer CI tool 🏴 #1402

Add primer CI tool 🏴 #1402

cooperlees commented May 10, 2020 •

edited

SanketDG left a comment

SanketDG May 14, 2020

cooperlees May 16, 2020

SanketDG May 14, 2020

cooperlees May 16, 2020

SanketDG May 14, 2020

cooperlees May 16, 2020

SanketDG May 14, 2020

cooperlees May 16, 2020

SanketDG May 16, 2020

SanketDG May 14, 2020

cooperlees May 16, 2020

SanketDG May 16, 2020

JelleZijlstra May 17, 2020

cooperlees May 17, 2020 •

edited

SanketDG May 14, 2020

SanketDG May 14, 2020

cooperlees May 16, 2020

cooperlees left a comment

cooperlees May 16, 2020

cooperlees May 16, 2020

cooperlees May 16, 2020

cooperlees May 16, 2020

cooperlees May 16, 2020

cooperlees May 16, 2020



		DEFAULT_CONFIG = Path(__file__).parent / "primer.json"
		DEFAULT_WORKDIR = Path(gettempdir()) / f"primer.{getpid()}"

		LOG = logging.getLogger(__name__)


		class Results(NamedTuple):

		LOG = logging.getLogger(__name__)


		class Results(NamedTuple):

Add primer CI tool 🏴 #1402

Add primer CI tool 🏴 #1402

Conversation

cooperlees commented May 10, 2020 • edited

SanketDG left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cooperlees May 17, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cooperlees left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cooperlees commented May 10, 2020 •

edited

cooperlees May 17, 2020 •

edited