Skip to content
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

RefreshMixin.refresh() doesn't remove removed attributes #1155

Closed
ericfrederich opened this issue Aug 20, 2020 · 2 comments · Fixed by #1213
Closed

RefreshMixin.refresh() doesn't remove removed attributes #1155

ericfrederich opened this issue Aug 20, 2020 · 2 comments · Fixed by #1213

Comments

@ericfrederich
Copy link
Contributor

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).attributes

Actual 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
@ericfrederich
Copy link
Contributor Author

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 with server_data as the parameter.
I'd suspect that we could simply modify the _update_attrs method to make it exactly equal.
I'm guessing this because it is only ever called with server_data so likely never called with any partial updates.

Current implementation

    def _update_attrs(self, new_attrs):
        self.__dict__["_updated_attrs"] = {}
        self.__dict__["_attrs"].update(new_attrs)

The .update(new_attrs) could be replaced with = new_attrs.

max-wittig added a commit that referenced this issue Oct 12, 2020
This fixes and error, where deleted attributes would not show up

Fixes #1155
max-wittig added a commit that referenced this issue Oct 12, 2020
This fixes and error, where deleted attributes would not show up

Fixes #1155
@max-wittig
Copy link
Member

This totally makes sense. There is no need to keep the old state

I created #1213 and added a test for it

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants