blender/blender-asset-tracer
2
5
Fork 5
Code Issues 4 Pull Requests 1 Projects Releases Wiki Activity

blocks2assets: debug filepath on reading unsupported blocks #92885

Merged
Sybren A. Stüvel merged 2 commits from oliverpool/blender-asset-tracer:blocks2assets_print_filepath into main 2023-12-18 17:57:44 +01:00
Conversation 5 Commits 2 Files Changed 1 +14 -3
Olivier C commented 2023-11-06 09:58:17 +01:00
Contributor
Copy Link

To ease support of other data blocks in the future, this PR logs the filepath to the debug message on unsupported block types (if present).

To test this PR, you need to have a blend file with an unsupported block type, which contains a filepath property.

For instance VDB volumes of scene_with_vdb_and_fluid_cache.zip are not supported yet (PR pending in #92884). As long as VDB are not supported by BAT, the log will read:

DEBUG:No expander implemented for block type 'VO'
DEBUG:No reader implemented for block type 'VO' (filepath='//cache_vdb/CCup_ws_01/fluid_data_0002.vdb')

When rendering a scene prepared by BAT, having the filepath of the block in the log will help a lot, to know what kind of asset could not be read.

To ease support of other data blocks in the future, this PR logs the filepath to the debug message on unsupported block types (if present). To test this PR, you need to have a blend file with an unsupported block type, which contains a `filepath` property. For instance VDB volumes of [scene_with_vdb_and_fluid_cache.zip](https://projects.blender.org/attachments/a1bdd002-d684-4709-bb0a-744590606413) are not supported yet (PR pending in #92884). As long as VDB are not supported by BAT, the log will read: ``` DEBUG:No expander implemented for block type 'VO' DEBUG:No reader implemented for block type 'VO' (filepath='//cache_vdb/CCup_ws_01/fluid_data_0002.vdb') ``` When rendering a scene prepared by BAT, having the filepath of the block in the log will help a lot, to know what kind of asset could not be read.
Olivier C added 1 commit 2023-11-06 09:58:18 +01:00
Sybren A. Stüvel commented 2023-12-14 10:35:57 +01:00
Member
Copy Link

Please update the PR description, so that it actually describes what the PR is for.

Please update the PR description, so that it actually describes what the PR is for.
Sybren A. Stüvel requested changes 2023-12-14 10:44:12 +01:00
Sybren A. Stüvel left a comment
Member
Copy Link

The code is getting quite nested now, which I think can easily be resolved. Also I would suggest using %r for printing the filepath, rather than %s, such that trailing spaces and empty strings are recognisable. Finally, it would help to know that the 2nd string that's printed is actually the filepath property.

This would combine into something like:

except KeyError:
    if block.code in _warned_about_types:
        return

    blocktype = block.code.decode()
    filepath = block.get(b"filepath", "", as_str=True)
    if filepath:
        log.debug(
            "No reader implemented for block type %r (filepath=%r)",
            blocktype,
            filepath,
        )
    else:
        log.debug("No reader implemented for block type %r", blocktype)
    _warned_about_types.add(block.code)
    return

Could you also add some instructions to the PR description on how to test this particular branch of the code? That'll make testing & accepting this PR a lot easier.

The code is getting quite nested now, which I think can easily be resolved. Also I would suggest using `%r` for printing the filepath, rather than `%s`, such that trailing spaces and empty strings are recognisable. Finally, it would help to know that the 2nd string that's printed is actually the `filepath` property. This would combine into something like: ```python except KeyError: if block.code in _warned_about_types: return blocktype = block.code.decode() filepath = block.get(b"filepath", "", as_str=True) if filepath: log.debug( "No reader implemented for block type %r (filepath=%r)", blocktype, filepath, ) else: log.debug("No reader implemented for block type %r", blocktype) _warned_about_types.add(block.code) return ``` Could you also add some instructions to the PR description on how to test this particular branch of the code? That'll make testing & accepting this PR a lot easier.
Olivier C force-pushed blocks2assets_print_filepath from 5088f6d569 to 589dbd6752 2023-12-15 10:05:12 +01:00 Compare
Olivier C commented 2023-12-15 10:08:24 +01:00
Author
Contributor
Copy Link

I implemented your suggestion and rebased on top of main (hence the force-push)

I implemented your suggestion and rebased on top of main (hence the force-push)
Sybren A. Stüvel commented 2023-12-15 11:55:56 +01:00
Member
Copy Link

Could you also add some instructions to the PR description on how to test this particular branch of the code? That'll make testing & accepting this PR a lot easier.

This one is still missing, though.

> Could you also add some instructions to the PR description on how to test this particular branch of the code? That'll make testing & accepting this PR a lot easier. This one is still missing, though.
Olivier C commented 2023-12-15 14:20:49 +01:00
Author
Contributor
Copy Link

This one is still missing, though.

Sorry, I overlooked this part of your comment.

You can use the .blend file of #92884: scene_with_vdb_and_fluid_cache.zip, the log will output:

DEBUG:No expander implemented for block type 'VO'
DEBUG:No reader implemented for block type 'VO' (filepath='//cache_vdb/CCup_ws_01/fluid_data_0002.vdb')
> This one is still missing, though. Sorry, I overlooked this part of your comment. You can use the .blend file of #92884: [scene_with_vdb_and_fluid_cache.zip](https://projects.blender.org/attachments/a1bdd002-d684-4709-bb0a-744590606413), the log will output: ``` DEBUG:No expander implemented for block type 'VO' DEBUG:No reader implemented for block type 'VO' (filepath='//cache_vdb/CCup_ws_01/fluid_data_0002.vdb') ```
Olivier C requested review from Sybren A. Stüvel 2023-12-15 17:28:42 +01:00
Sybren A. Stüvel merged commit 2099827554 into main 2023-12-18 17:57:44 +01:00
Sybren A. Stüvel deleted branch blocks2assets_print_filepath 2023-12-18 17:57:53 +01:00
Sybren A. Stüvel commented 2023-12-18 17:58:01 +01:00
Member
Copy Link

Looks good, thanks!

Looks good, thanks!
Sign in to join this conversation.
Reviewers
No reviewers
Sybren A. Stüvel
Labels
Clear labels   
legacy module
Platforms, Builds, Tests & Devices

  
legacy project
Blender Asset Tracer

  
legacy project
Platform: Windows

  
Priority::Normal

   
Status::Archived

   
Status::Confirmed

   
Status::Needs Triage

   
Status::Resolved

   
Type::Design

   
Type::Report

   
Type::To Do
No Label
legacy module
Platforms, Builds, Tests & Devices
legacy project
Blender Asset Tracer
legacy project
Platform: Windows
Priority::Normal
Status::Archived
Status::Confirmed
Status::Needs Triage
Status::Resolved
Type::Design
Type::Report
Type::To Do
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender-asset-tracer#92885
No description provided.
Delete Branch "oliverpool/blender-asset-tracer:blocks2assets_print_filepath"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?