updates made ..

updated utils.py _validate_attrs() .. it was unintentionally indented.

looks like utils.py doesn't know type RequiredOptional used in _validate_attrs param types? does utils.py need "from gitlab import base"?

added base to imports (from gitlab import types, base)

Thanks for your patience@walterrowe, getting closer now ;)

I suggest you always run a quicktox before pushing, this way the linters will already let you know if something's wrong or if the tests don't pass.

added base to imports (from gitlab import types, base)

Thanks for this. Our linters are quite strict so likely it'll need to be alphabetical, sorry 😁

looks like utils.py doesn't know type RequiredOptional used in _validate_attrs param types? does utils.py need "from gitlab import base"?

You're right, sorry I missed this when I looked at the PR. BecauseRequiredOptional is inbase.py, we also now get a circular import (we import base from utils and vice versa), so it would probably make sense to haveRequiredOptional defined inutils.py (but still import it in base e.g.from .utils import RequiredOptional otherwise you'll need to touch a bunch of files). I'll refactor that later if needed.

looked at the error .. looking for suggestions on how to resolve

I followed the contributors doc and set up all the pre-commit items. I have addressed them all except these two.

gitlab/utils.py:168: error: Name "RequiredOptional" is not definedgitlab/mixins.py:345: error: Item "None" of "Optional[Type[RESTObject]]" has no attribute "_id_attr"

utils.py where we declare attributes arg type

def _validate_attrs(    data: Dict[str, Any],    attributes: RequiredOptional,    excludes: Optional[list] = None,) -> None:

mixins.py where we set excludes

        excludes = [self._obj_cls._id_attr]        utils._validate_attrs(            data=new_data, attributes=self._update_attrs, excludes=excludes        )

In utils.py ...

from gitlab.base import RequiredOptional

This resolves the RequiredOptional type error and the circular imports between base and utils.

I tried moving RequiredOptional from base to utils and two dozen other files then said they didn't know RequiredOptional so moving it is quite involved. Looking at the groups.py object file I see it tries to import RequiredOptional from base (from gitlab.base import RequiredOptional, RESTManager, RESTObject) so I took the same approach in utils.py.

@nejch - I still get pre-commit complaining about _id_attr in the update method of UpdateMixins in mixins.py:

gitlab/mixins.py:345: error: Item "None" of "Optional[Type[RESTObject]]" has no attribute "_id_attr"

It comes from this line:

        excludes = [self._obj_cls._id_attr]

I moved this ref from the old _check_missing_update_attrs() method.

        required = tuple(            [k for k in self._update_attrs.required if k != self._obj_cls._id_attr]        )

I need help with that one. Tracing backwards it appears _id_attr would be inherited from RESTObject, its optional, and defaults to "id". Perhaps in the update method of the UpdateMixin we don't need this, or need to first check if its not None?

added check for self._obj_cls is not None .. pre-commit runs clean

@nejch .. now I'm stuck .. any guidance would be helpful.

Just jumping in here without doing a full review. It might make sense to moveRequiredOptional togitlab/types.py.

Asgitlab/types.py has no dependencies on any othergitlab module. This should get rid of the circular dependency issue.

Thanks@JohnVillalovos that makes sense.

@walterrowe you can move it totypes.py and re-import inbase.py (andutils.py) for now so that you don't need to touch all the other files.

Thanks@JohnVillalovos that makes sense.

@walterrowe you can move it totypes.py and re-import inbase.py (andutils.py) for now so that you don't need to touch all the other files.

I created a PR for moving theRequiredOptional as it was not so easy.

#2039

@walterrowe the PR above was merged, so if you now rebase your PR on top of our main you should be able to do this pretty easily, just import it fromtypes.py. Thanks! :)

@walterrowe the PR above was merged, so if you now rebase your PR on top of our main you should be able to do this pretty easily, just import it fromtypes.py. Thanks! :)

If you need assistance rebasing please let us know. I may be busy today, but starting tomorrow I will have more free time.

@walterrowe the PR above was merged, so if you now rebase your PR on top of our main you should be able to do this pretty easily, just import it fromtypes.py. Thanks! :)

If you need assistance rebasing please let us know. I may be busy today, but starting tomorrow I will have more free time.

Sure wish I had seen this before I pushed my commit to update almost all those same files with the same changes. I have the pre-commit setup so it warned me about all of them.

@JohnVillalovos - I might need some assistance at this point since I have pushed my commits to my fork associated with the PR.

@JohnVillalovos - I might need some assistance at this point since I have pushed my commits to my fork associated with the PR.

Give me a few minutes and I will attempt to rebase.

Give me a few minutes and I will attempt to rebase.

@walterrowe I rebased and squashed the PR. I have pushed it to your branch. You will need to re-clone it most likely.

Please let me know if you have questions.

Might be easiest to just do a new:
git clone git@github.com:walterrowe/python-gitlab.git

who would have thought implementing mutually exclusive attributes would require this much effort! thanks for all the assistance and guidance through this. definitely learning a great deal about python and code review that I didn't know before.

fixed the test_mixins_methods refs to _check_missing_create_attrs and _check_missing_update_attrs

Codecov Report

Merging#2037 (5bf017a) intomain (37eb8e0) willdecrease coverage by0.09%.
The diff coverage is75.86%.

@@            Coverage Diff             @@##             main    #2037      +/-   ##==========================================- Coverage   93.78%   93.68%   -0.10%==========================================  Files          78       78                Lines        4984     4985       +1     ==========================================- Hits         4674     4670       -4- Misses        310      315       +5

Flags with carried forward coverage won't be shown.Click here to find out more.

@walterrowe Oh I forgot one thing. Can you "squash" it down to one commit?

I can do it for you if you like.

Basic idea is:

git rebase --interactive HEAD~4

Change the last three lines frompick tosquash and edit the commit message.

@JohnVillalovos - I think I successfully did this .. double check?

@JohnVillalovos - I think I successfully did this .. double check?

Squash was successful 👍

Can you do agit commit --amend and edit the commit message?

feat: add support for mutually exclusive attributes, consolidate attribute validation, fix boards.py _create_attradd exclusive tuple to RequiredOptional data class to support formutually exclusive attributesconsolidate _check_missing_create_attrs and _check_missing_update_attrsfrom mixins.py into _validate_attrs in utils.pymoved RequiredOptional from base.py to types.py  (DELETE THIS) **************************************************change _create_attrs in board list manager classes fromrequired=('label_ld',) toexclusive=('label_id','asignee_id','milestone_id')closes https://github.com/python-gitlab/python-gitlab/issues/1897DELETE ALL BELOW HERE: **********************************************************************************fix: check attributes.require and .exclusive before using in _validate_attrs (utils.py)style: recommended changes in PR reviewfix: corrected _check_missing refs to use _validate_attrs refs

commit message amended

thanks@JohnVillalovos

is this now pending@nejch review?

Thanks for all the work on this@walterrowe! I had some thoughts on the logic for optionals as we said and I'd like to add some tests for this, but I'll do that in a follow-up, let's get this merged :)