Make source checkout steps not rely on hard-coded list of modes #1573
Conversation
An "attribute group" is a group of attributes with a common name prefix. The "mode" attribute group contains two members: full and incremental. The full and incremental names must have the same "mode_" prefix, making the method names "mode_full" and "mode_incremental". The _hasAttrGroupMember and _getAttrGroupMember are the attribute group equivalents of hasattr and getattr. The _listAttrGroupMembers exists so options can be returned in error messages to help people know what the valid options are without checking the docs or source.
Also added validation of the mode when creating the step. No other validation was added.
The check if self.mode is in possible_modes can be changed but there's also a check if it's a renderable. That's different from everything else so I am skipping it until the history is better understood.
|
Of course I skipped running all of the tests and see some integration tests failed. It appears the test is wrong because it's using |
Per the documentation, mode is defined to only accept "full" or "incremental". The method parameter is what accepts "copy". It is an old commit that set the integration test to use mode="copy" and the CVS step does little checking.
| Returns a list of all members in the attribute group. | ||
| """ | ||
| from inspect import getmembers, ismethod | ||
| methods = getmembers(self, lambda m: ismethod) |
There was a problem hiding this comment.
Hmm? Parameter m is not used?
Did you mean to just use ismethod?
|
Re |
|
@sa2ajj Not sure I understand your question. Which function should have a parameter? |
|
currently you have |
|
As docs states: |
|
My primary objection is the inability to verify |
|
It was just a question. Anyway, what else needs to be done in this PR so it can land? |
|
I don't know. You are the one that added the "needs work" label :). |
|
My label was about use of I'll have a look now. |
|
Ah, wait, we've been discussing moving that functionality to the base class. Do you think it's for the next PR? |
|
I think so. The next PR can figure out what to do about |
Make source checkout steps not rely on hard-coded list of modes
Updated all of the source checkout steps that have a
modeoption to query their methods to determine the available modes. I dubbed this an "attribute group". This pull request only touches themodeparameter; themethodparameter is another that could benefit from this.Note that I skipped fully converting the p4 step due to a different check of
self.mode.This pull request supersedes #1458.