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

[deviantart] fix crash when handling deleted deviations in status updates #3656

Merged
merged 2 commits into from
Feb 18, 2023

Conversation

ClosedPort22
Copy link
Contributor

@ClosedPort22 ClosedPort22 commented Feb 15, 2023

Example:

"items": [
    {
        "type": "deviation",
        "deviation": {
            "deviationid": "147C8B03-7D34-AE93-9241-FA3C6DBBC655",
            "printid": null,
            "is_deleted": true
        }
    }
],

This deviation isn't actually deleted and can be fetched as usual using the deviation endpoint.

I've been working on several features and bugfixes for DA extractors and this is one of them. Unfortunately they are heavily intertwined with each other, so I'm trying my best to separate them into branches and reduce the overall complexity.

Known issues:

  • If there are sta.sh-ed deviations in items and extra is enabled, these deviations will be extracted twice
  • metadata and folders doesn't work for deviations in items
  • If there are duplicate deviations, the deviation endpoint will be called multiple times

Currently WIP features/bugfixes that will address these issues or affect the implementation of this fix:

@rautamiekka
Copy link
Contributor

If there are sta.sh-ed deviations in items and extra is enabled, these deviations will be extracted twice
If there are duplicate deviations, the deviation endpoint will be called multiple times

We could just add a flag (or >=2, if necessary) to each deviation ID to know which ones are already downloaded/whatever, which is a drop in the ocean in RAM usage. If the flag is missing or equals "not downloaded" (or whatever the precise wording is), skip it.

@ClosedPort22
Copy link
Contributor Author

If there are sta.sh-ed deviations in items and extra is enabled, these deviations will be extracted twice
If there are duplicate deviations, the deviation endpoint will be called multiple times

We could just add a flag (or >=2, if necessary) to each deviation ID to know which ones are already downloaded/whatever, which is a drop in the ocean in RAM usage. If the flag is missing or equals "not downloaded" (or whatever the precise wording is), skip it.

This has already been fixed, it's just not part of this PR. It's actually more complicated than that because dA's metadata endpoint doesn't like duplicate UUIDs, and I ended up deduplicating them using a dict before further processing.

The first issue was fixed by using seen = set(), similar to other extractors.

Copy link
Owner

@mikf mikf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you and my condolences for having to deal with DA's inner workings yet again. Why oh why does this site have to be so complicated?

Comment on lines 1493 to 1512
if results:
if "deviationid" in results[0]:
if self.metadata:
self._metadata(results)
if self.folders:
self._folders(results)
else:
# attempt to fix "deleted" deviations
for result in results:
for item in result.get("items") or ():
if "deviation" not in item or \
not item["deviation"]["is_deleted"]:
continue
patch = self._call(
"/deviation/" +
item["deviation"]["deviationid"],
fatal=False)
if patch:
item["deviation"] = patch

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought, but maybe it'd be better to use a separate _pagination method for status items? Or at least put fixing deleted deviations in its own method.

I admit to having done this myself (plenty of times?) , but it hurts even me seeing ~50 spaces of indentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation here will be different if combined with this branch, which is almost done (I think). I'll try to refactor this section to make it look nicer though.

Comment on lines 1506 to 1509
patch = self._call(
"/deviation/" +
item["deviation"]["deviationid"],
fatal=False)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use self.deviation()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now (at least on this branch) neither metadata nor folders is supposed to work for shared deviations, and I'd rather not make an exception for "deleted" deviations. This will be fixed by this branch, which uses _call to bypass _metadata and _folders to make API calls more efficient.

@mikf mikf merged commit b4899c2 into mikf:master Feb 18, 2023
@ClosedPort22 ClosedPort22 deleted the da-status-deleted-fix branch February 18, 2023 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants