Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Managed Jenkins Infrastructure for TVM RFC #49
Add Managed Jenkins Infrastructure for TVM RFC #49
Changes from 2 commits
f5f8b1a
1226950
8d2e052
c835187
aca84fe
33b5899
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Part of the problem with current CI is that even as a TVM committer I can't make meaningful changes to the infrastructure. The infrastructure in itself is a part of the TVM project, I'd suggest we encourage people to contribute infrastructure-as-code similar to other contributions, by using the committer system, rather than an alternative one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i agree with this. i think there are a few more obstacles before we can do this and i'd like to solve them in parallel without blocking efforts to improve CI:
what we're proposing is to handle this separately for now, however still grant TVM committers write access to the IaC repo (so the system is essentially still the committer system, just with extra folks who can write/deploy). this will also give us a good idea as to the GH permissions needed for such a repo, so that we can then consider unifying the two systems with a proper proposal later on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's also some prior art of CI code living outside the main repo (see https://github.com/kubernetes/test-infra, https://github.com/pytorch/builder), afaik for similar reasons (easier to commit to and iterate on)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify, I don't have any issue with the actual code being in a separate respository with different checks and such, that's totally normal in a lot of projects. The point of concern is taking the CI infrastructure, which every commit into TVM depends on, outside of the Apache TVM project. Taking your examples of
kubernetes/test-infra
andpytorch/builder
, they all exist within the project itself, so the Kubernetes CI is under thekubernetes
namespace and governed under those rules.I'm not sure this is true, I believe that the TVM community has a reasonable number of active committers comfortable with reviewing both, it's historically been difficult for them to contribute and continuing to manage it outside of the project seems to continue that practice. The path to becoming a committer does not seem to require comprehensive of knowledge across TVM, as the code owners file demonstrates certain committers have a large preference to a single area. I would support the PMC in guiding those who are interested in solely contributing to CI to becoming comitters as much as those who would contribute to other areas, such as documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs rephrasing with the context around choosing
tlcpack
and using TVM Committers?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i removed the last sentence, but i'd prefer to leave the rest in here actually. i'm very much okay with proceeding using the existing committer promotion strategy--i don't believe we should make a process exception over a perceived fear that the process won't work. however, i think it is pretty plain that a multi-week PMC vote is an extremely heavyweight way to add folks to an oncall rotation. i don't consider this problem perfectly solved in the new system, so i don't think it's worth removing the critique that GH actions locks us into that problem. i would like to revisit this problem in context of real experience operating the IaC repo and motivate any process changes off of that rather than off of a gut feeling. this does mean we're proceeding with a degraded oncall support, but i'm okay with that given it's a CI and the goal is to build a community-driven process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to these it'd be nice if we had a single guide on how to deploy everything (from the level of I have a head node and some static machines with a freshly provisioned Ubuntu or something), both for ourselves in the future and to enable this as more than just a possibility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a great idea--I'll definitely look into doing this in the coming weeks.