Add contrib driver for Hashicorp Nomad jobs #3103
Conversation
There was a problem hiding this comment.
Just left a few code comments. Requests for some improved readability.
Also, Github is complaining that this branch isn't up-to-date with master. Could you update it?
Lastly, looks like you've got 1 flake8 complaint
./test/contrib/nomad_test.py:66:5: E265 block comment should start with '# '
| region=None, namespace=self.nomad_namespace, token=None) | ||
| self.job_uuid = str(uuid.uuid4().hex) | ||
| now = datetime.utcnow() | ||
| self.uu_name = "%s-%s-%s" % (self.name, now.strftime('%Y%m%d%H%M%S'), self.job_uuid[:16]) |
There was a problem hiding this comment.
could we use newer str.format or f-strings? I find them easier to read than % style interpolation or + concatenation
There was a problem hiding this comment.
This part was just copied from the kubernetes plugin
luigi/contrib/nomad.py
Outdated
| job_json['Job']['Meta'].update(labels) | ||
| if self.nomad_namespace is not None: | ||
| job_json['Namespace'] = self.nomad_namespace | ||
| if "Datacenters" not in job_json["Job"]: | ||
| job_json["Job"]["Datacenters"] = ["dc1"] | ||
| if "TaskGroups" in job_json["Job"]: | ||
| for tg in job_json["Job"]["TaskGroups"]: | ||
| if "RestartPolicy" not in tg: | ||
| tg["RestartPolicy"] = { |
There was a problem hiding this comment.
There are lots of default settings in here. Is there a more explicit way to specify a default job_json with all these various default and then update the nested json with user-defined specifics?
If the code to do that is just as involved with lots of conditionals, don't worry about it. But reading through this, I find it less than easy to interpret what the total of defaults are if i were to not specify various overrides.
There was a problem hiding this comment.
I divided it up a bit further, that should hopefully make it more self-explanatory.
It looks a bit complex just because that it respects any user spec, not overriding.
Based on the Kubernetes contrib, but different.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If closed, you may revisit when your time allows and reopen! Thank you for your contributions. |
Based on the Kubernetes contrib, but different.
Closes #3102