Add CICD schema and render workflows #1068
Conversation
|
@costrouc this is what I have so far, I still have a few things to wrap up but I'm happy with this outline so far. Let me know if you have questions, concerns or suggestions :) Outstanding:
|
|
@costrouc workflow files being deprecated:
|
|
Originally I was going to say we should convert this to just yaml like syntax instead of sticking with json which can be interpreted as yaml. But the json is extremely readable and I think we should stick with it since it is less ambiguous. I do see the future concern tha the |
|
I also see the version issue on |
|
@costrouc keeping it in json is fine with me! |
|
I'm fine for now just merging this work and leaving it to another PR to figure this out for development. For an actual Pypi release this won't be a problem |
There was a problem hiding this comment.
Amazing, the fact that we are using schema for it makes a good difference when testing in the future. Amazing work @iameskild, have you tested the gitlab actions?
| branch = config["ci_cd"]["branch"] | ||
| qhub_version = config["qhub_version"] | ||
|
|
||
| push = GHA_on_extras(branches=[branch], path=["qhub-config.yaml"]) |
There was a problem hiding this comment.
are we considering #995 here?
There was a problem hiding this comment.
I didn't see this but I think won't be too hard. Do you think we should include it here?
There was a problem hiding this comment.
I think so, as it was linked to the original issue and it would be just adding an if for step5 based on this new variable
| @@ -349,3 +349,15 @@ def deep_merge(*args): | |||
| return [*d1, *d2] | |||
| else: # if they don't match use left one | |||
| return d1 | |||
|
|
|||
|
|
|||
| def pip_install_qhub(qhub_version: str) -> str: | |||
There was a problem hiding this comment.
@costrouc this is an unsophisticated way of handling the qhub_version issue for now. Would this be okay to add?
There was a problem hiding this comment.
@iameskild looks great to me. After merging this we can test this and impove upon it when we find issues.
Fixes | Closes | Resolves #1013
Changes:
Much of this code was inspired by our existing
schema.pyand from conversations and discussion had with @costrouc and @tonyfast. Thank you both for your assistance and feedback.Types of changes
What types of changes does your code introduce?
Put an
xin the boxes that applyTesting
Requires testing
In case you checked yes, did you write tests?
Not yet...
Further comments (optional)
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered and more.