Skip to content

Refactor/string paths storage rtdb#19

Merged
jckenny59 merged 12 commits intomainfrom
refactor/string-paths-storage-rtdb
Mar 27, 2026
Merged

Refactor/string paths storage rtdb#19
jckenny59 merged 12 commits intomainfrom
refactor/string-paths-storage-rtdb

Conversation

@jckenny59
Copy link
Copy Markdown
Collaborator

What type of change is this?

  • Bug fix in a backwards-compatible manner.
  • New feature in a backwards-compatible manner.
  • Breaking change: bug fix or new feature that involve incompatible API changes.
  • Other (e.g. doc update, configuration, etc)

Summary
This PR refactors Firebase access to a path-based API across RealtimeDatabase and Storage, and updates ProjectManager to use the new path-based calls.

Key changes:

  • RealtimeDatabase now uses path-based operations as the primary API.
  • Storage now uses path-based operations as the primary API.
  • ProjectManager call sites were migrated away from removed deep/child/list helper methods.
  • Internal shared path handling was consolidated in the internal path utility module.
  • Manual smoke checks were expanded in tests/random_testing.py, including byte-upload scenarios.

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I added a line to the CHANGELOG.md file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
  • I ran all tests on my computer and it's all green (i.e. invoke test).
  • I ran lint on my computer and there are no errors (i.e. invoke lint).
  • I added new functions/classes and made them available on a second-level import, e.g. compas.datastructures.Mesh.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added necessary documentation (if appropriate)

Notes
This PR is marked as breaking because legacy child/deep/list APIs were removed in favor of path-based APIs.
Callers now need to use path strings with the new method surface.

Affected components:

  • Realtime database layer: realtime_database.py
  • Storage layer: storage.py
  • Shared path utilities:_path.py
  • Project integration:
    project_manager.py

Validation scope:
I did not run the full project test suite or lint suite yet (for example invoke test / invoke lint).

This is simply updating the RealtimeDatabase class to take paths instead the previous API for children and reference names.
This is updating to a path based reference other than making something that uploads the previous child based methods.
This includes updating the Project Manager class to the path based referencing.
@jckenny59 jckenny59 requested a review from gonzalocasas March 13, 2026 23:47
@gonzalocasas
Copy link
Copy Markdown
Member

@jckenny59 Could you please change base to prep-release branch, so that the diff looks correct?

@jckenny59 jckenny59 changed the base branch from main to prep-release March 14, 2026 16:42
jckenny59 and others added 4 commits March 14, 2026 12:52
This is updating the method signatures in the overall class. As most of this had to be undone in the conflict resolution.
This is simple as well, and does the same to update method signatures in because of conflict resolution.
@jckenny59
Copy link
Copy Markdown
Collaborator Author

@gonzalocasas taken care of, and updated method formatting after my merge conflict :)

Base automatically changed from prep-release to main March 16, 2026 07:05
Copy link
Copy Markdown
Member

@gonzalocasas gonzalocasas left a comment

Choose a reason for hiding this comment

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

lgtm + a few smaller details here and there

Comment on lines +161 to +163
# TODO: Check if this is stupid... it provides the functionality of making it work with compas objects and consistency across both child classes
json_string = json_dumps(data)
database_reference.set(json.loads(json_string))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this needed?

@jckenny59 jckenny59 merged commit 809d9a6 into main Mar 27, 2026
13 checks passed
@jckenny59 jckenny59 deleted the refactor/string-paths-storage-rtdb branch March 27, 2026 15:43
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.

2 participants