Repository navigation
RefreshMixin.refresh() doesn't remove removed attributes #1155
Description
Description of the problem, including code/CLI snippet
When attributes disappear from an object on the server RefreshMixin.refresh() doesn't remove them.
For instance if a job that has artifacts will have an artifacts_file attribute. If you call .delete_artifacts() on it, then call .refresh() the artifacts_file attribute will still be there.
# get a job with artifacts
job = project.jobs.get(job_id)
# will succeed
assert hasattr(job, "artifacts_file")
# now delete the artifacts from the server
job.delete_artifacts()
# This will fail because the artifacts_file is still there; refresh() didn't remove it
job.refresh()
assert not hasattr(job, "artifacts_file")
# If you get the job again from the project it'll be fine
job = project.jobs.get(job_id)
assert not hasattr(job, "artifacts_file")Expected Behavior
I would expect that the attributes dict on any object should be exactly the same between a freshly retrieved object and an old object after calling .refresh()
o.refresh()
# After a refresh this should always be true
o.attributes == o.manager.get(o.id).attributesActual Behavior
They're not equal
Specifications
- python-gitlab version:
v2.4.0 - API version you are using (v3/v4):
v4 - Gitlab server version (or gitlab.com):
13.2.3
Activity
ericfrederich commentedon Aug 20, 2020
Before creating a merge request I'd like to know if we feel this is indeed a bug.
Doing a
git grep "\._update_attrs(" "*.py"I see that this method is only ever called withserver_dataas the parameter.I'd suspect that we could simply modify the
_update_attrsmethod to make it exactly equal.I'm guessing this because it is only ever called with
server_dataso likely never called with any partial updates.Current implementation
The
.update(new_attrs)could be replaced with= new_attrs.fix(base): really refresh object
fix(base): really refresh object
max-wittig commentedon Oct 12, 2020
This totally makes sense. There is no need to keep the old state
I created #1213 and added a test for it