Begin versioning Assigner configs by LinuxMercedes · Pull Request #111 · redkyn/assigner

LinuxMercedes · 2018-02-02T04:45:34Z

Add schemas for versions 0, 1, and 2
Schema validation
Upgrading configs from one version to the next
Bump GitPython from 1.0.1 to 2.1.8 (fixes PackingType of packed-Refs not understood: '# pack-refs with: peeled fully-peeled sorted' gitpython-developers/GitPython#687)

TODO:

Changelog
Upgrade assigner to use v2 config

LinuxMercedes · 2018-02-02T05:44:16Z

+            "name": "gitlab",
+            "token": "xxx gitlab token xxx",
+            "host": "https://git.gitlab.com",
+        },


@brhoades What do you think of this configuration schema? If you like it, I'll update the rest of the code to work with it.

The schema will go something like this: the backend field is a JSON object with at least one key, name. The remainder of the data will be something that we can pass to the Backend class and it will be able to pull what it needs out of that.

So, for the mock backend, the config would look like

"backend" : { "name" : "mock" }

Seem sensible?

I want to make sure I understand you right. Are you referring to the schema here, where we double check the functions are properly enforcing schema version differences? Or do you mean in general :D?

I mean for the newest schema, in preparation for #102 😄

This looks good. Will allow nonbreaking backend additions.

Cool -- in that case I'll propagate the change through BaseRepo et. al.

brhoades · 2018-02-07T00:06:01Z

@@ -0,0 +1,282 @@
+
+SCHEMAS = [


I worry this would get unruly fast. It's less rigid but would make more sense to define INITIAL_SCHEMA, make that version 0/1, then copy it and modify what changed in two as part of the script. Then you only need to read blocks of code to determine changes rather than hunting for comments.

You've got copy.deepcopy if you agree ;).

I fear that writing diff functions rather than schemas will make it harder to write schemas (the proposed version 2 schema required some troubleshooting) and that it'll slow down startup -- if we could constexpr all of this that would be great...

That's fine with me. I just feel there's a better way to orgfanize this. Maybe different files for each schema version?

Yeah, we can do that!

brhoades · 2018-02-07T00:07:48Z

+from assigner.config.upgrades import UPGRADES
+
+logger = logging.getLogger(__name__)
+


np: 2 spacesssssssssssssssssssssssssss
We need that flake8 linter.

brhoades · 2018-02-07T00:08:07Z

+
+
+def validate(config, version=(len(SCHEMAS) - 1)):
+    assert version < len(SCHEMAS)


I've never seen this. I noticed it was a builtin but this makes sense.

Yeah! I should rename this to _validate or rewrite this to throw...going to do the former.

brhoades · 2018-02-07T00:08:26Z

+    pass
+
+
+def validate(config, version=(len(SCHEMAS) - 1)):


You can put expressions for parameter defaults?

Apparently so!

brhoades · 2018-02-07T00:12:14Z

+    def test_validate(self):
+        for version, config in enumerate(CONFIGS):
+            try:
+                validate(config, version)


Two alternatives:

with self.assertRaises(ValidationError): validate(config, version)

or

self.assertRaises(validate, config, version)

I prefer the former.

Wait, I want the opposite of assertRaises. Does such a thing exist?

Oh, yeah, just run the function :DD. If an exception is thrown, BAM. Assuming you're doing try: ... except: ... self.fail, that's fine.

brhoades · 2018-02-07T00:13:59Z

+            "name": "gitlab",
+            "token": "xxx gitlab token xxx",
+            "host": "https://git.gitlab.com",
+        },


I want to make sure I understand you right. Are you referring to the schema here, where we double check the functions are properly enforcing schema version differences? Or do you mean in general :D?

LinuxMercedes · 2018-02-07T01:07:42Z

+    for version in range(current, latest):
+        config = UPGRADES[version](config)
+        assert get_version(config) == version + 1
+        _validate(config, version)


So, validation is a tricky thing here. Ideally we'd like to detect when a config goes from valid -> invalid, but in the case that a config is invalid to begin with, what do we do? The primary case of an invalid config is when assigner is run with no _config.yml and defaults to an empty config.

I suppose we could require that upgrade functions have to succeed even if they're handed a malformed config (which is what they do right now) and test that upgrading an empty config succeeds (which is what we do right now).

brhoades · 2018-02-09T14:46:52Z

@@ -0,0 +1,63 @@
+import logging


My merge conflict sense is tingling.

- Add schemas for versions 0, 1, and 2 - Schema validation - Upgrading configs from one version to the next

Fixes gitpython-developers/GitPython#687

LinuxMercedes changed the title ~~Begin versioning Assigner configs~~ [WIP] Begin versioning Assigner configs Feb 2, 2018

LinuxMercedes force-pushed the pr/versioned-config branch 2 times, most recently from 13a2d49 to ce993b6 Compare February 2, 2018 04:55

LinuxMercedes commented Feb 2, 2018

View reviewed changes

brhoades mentioned this pull request Feb 2, 2018

Added abstract backend base with Gitlab implementation #102

Merged

LinuxMercedes force-pushed the pr/versioned-config branch from ccfbc36 to 7c8fada Compare February 4, 2018 21:34

brhoades reviewed Feb 7, 2018

View reviewed changes

LinuxMercedes force-pushed the pr/versioned-config branch from 7c8fada to 33d832b Compare February 7, 2018 01:03

LinuxMercedes commented Feb 7, 2018

View reviewed changes

LinuxMercedes force-pushed the pr/versioned-config branch 7 times, most recently from 47b36e1 to 75204e9 Compare February 9, 2018 05:24

LinuxMercedes changed the title ~~[WIP] Begin versioning Assigner configs~~ Begin versioning Assigner configs Feb 9, 2018

brhoades reviewed Feb 9, 2018

View reviewed changes

brhoades previously approved these changes Feb 9, 2018

View reviewed changes

LinuxMercedes dismissed brhoades’s stale review via bf24a1d February 9, 2018 17:37

brhoades previously approved these changes Feb 9, 2018

View reviewed changes

LinuxMercedes added 7 commits February 9, 2018 17:44

Begin versioning Assigner configs

20f661c

- Add schemas for versions 0, 1, and 2 - Schema validation - Upgrading configs from one version to the next

Logging messages for upgrades and for too-new configs

2d35953

Validate config during upgrades if possible

1bf7aeb

Bump GitPython to 2.1.8

ade259c

Fixes gitpython-developers/GitPython#687

Config v2: Make backend config generic

c18ba07

Update changelog

1942692

Split schemas into individual files

60308fd

LinuxMercedes dismissed brhoades’s stale review via 60308fd February 9, 2018 23:48

LinuxMercedes force-pushed the pr/versioned-config branch from bf24a1d to 60308fd Compare February 9, 2018 23:48

brhoades approved these changes Feb 10, 2018

View reviewed changes

LinuxMercedes merged commit 1338df9 into master Feb 11, 2018

LinuxMercedes deleted the pr/versioned-config branch February 11, 2018 00:19

LinuxMercedes added this to the v2.0 milestone Dec 22, 2018

		from assigner.config.upgrades import UPGRADES

		logger = logging.getLogger(__name__)



		def validate(config, version=(len(SCHEMAS) - 1)):
		assert version < len(SCHEMAS)

Conversation

LinuxMercedes commented Feb 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LinuxMercedes Feb 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

LinuxMercedes commented Feb 2, 2018 •

edited

Loading

LinuxMercedes Feb 4, 2018 •

edited

Loading