-
-
Notifications
You must be signed in to change notification settings - Fork 393
Update ADC to enable users to aquire paths for the active model #2843
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
Conversation
Added 2 functions to: 1- Get the organization name (ACC Hub). 2- Build a local path for the active model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary:
This PR adds two functions to the ADC library:
_get_organization_name()
: Extracts the ACC Hub organization name from ADC pathsget_model_path()
: Builds a local path for the active model including the organization folder
The goal is to enable users to acquire local paths for the active document itself, not just linked files.
Review Summary:
Found 4 critical bugs that need fixing before merge:
- Severe performance issue using
os.walk()
to traverse entire workspace - IndexError risk with unchecked array access
- Logic error calling function before validating required parameter
- Reversed conditional logic with nested if-statements
Also found Black formatting violations (as noted in PR checklist requirements).
The core idea is valuable but the implementation has several bugs that will cause crashes and poor performance in production.
Follow-up suggestions:
file_name = parts[1] | ||
drv_local_path = op.normpath(drv_info.WorkspaceLocation) | ||
subdirs = os.walk(drv_local_path) | ||
for root, dirs, files in subdirs: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical Performance Issue: os.walk()
traverses the entire ADC workspace directory tree which can be extremely slow for large ACC/BIM360 projects with many files and folders. This will cause significant delays when called.
The organization name should be extractable from the ADC API or the path structure itself without filesystem traversal. Consider using adc.GetDrives()
properties or parsing the workspace location structure directly.
If the org name is consistently in a specific directory level under WorkspaceLocation
, use direct path parsing instead:
def _get_organization_name(drv_info, path):
"""Get the organization name from the ADC path."""
drive_schema = ADC_DRIVE_SCHEMA.format(drive_name=drv_info.Name)
# Get relative path from drive schema
rel_path = path.replace(drive_schema, "").lstrip('/')
# Parse to get project/folder structure
parts = rel_path.split('/')
if len(parts) < 2:
return None
# Build expected local path based on known ADC structure
# This assumes organization name is first level dir in workspace
drv_local_path = op.normpath(drv_info.WorkspaceLocation)
subdirs = [d for d in os.listdir(drv_local_path)
if op.isdir(op.join(drv_local_path, d))]
# Return first matching org directory
if subdirs:
return subdirs[0]
return None
actions
Feedback: Rate this comment to help me improve future code reviews:
- 👍 Good - Helpful and accurate
- 👎 Poor - Wrong, unclear, or unhelpful
- Skip if you don't have any strong opinions either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately, there isn't a known way to parse the organization name through the API.
also there adc path extracted from the file, shows Accdocs://ProjectName/FileName.rvt. there is no way to quickly acquire organization name by deconstructing the path.
so far my only way to access is to look into the subfolders to find the target file and report the organization folder that contains the file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sanzoghenzo any Idea here?
@jmcouffin |
Noted. |
Added 2 functions to:
1- Get the organization name (ACC Hub).
2- Build a local path for the active model.
Description
The ADC library enables users to acquire local paths for files linked to the model, but not the active doc itself, this PR will attempt at solving it.
Checklist
Before submitting your pull request, ensure the following requirements are met: