Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

General: Move change context functions #2839

Merged

Conversation

iLLiCiTiT
Copy link
Member

Brief description

Context change related functions moved from avalon into openpype.

Description

These functions recalculate session changes and change of workdir used primarily in workfiles tool.

Changes

  • moved functions into openpype/lib/avalon_context.py - at this moment there is not a better place
  • changed how fusion is getting wordir path
  • use functions in workfiles tool

Testing notes:

  1. Workfiles tool should work as before
    • changes context on save
    • calculate proper workdir and uses right template
  2. Fusion scripts handling workdir should work - not sure if it did work before?

@mkolar
Copy link
Member

mkolar commented Mar 4, 2022

Task linked: OP-2844 Move change context functions

@iLLiCiTiT iLLiCiTiT self-assigned this Mar 4, 2022
@iLLiCiTiT iLLiCiTiT added type: enhancement Enhancements to existing functionality backend labels Mar 4, 2022
Comment on lines 48 to 51
def _get_work_folder(session):
"""Convenience function to get the work folder path of the current asset"""

# Get new filename, create path based on asset and work template
template_work = self._project["config"]["template"]["work"]
work_path = pipeline._format_work_template(template_work, session)

return os.path.normpath(work_path)
return get_workdir_from_session(session)
Copy link
Collaborator

@BigRoy BigRoy Mar 4, 2022

Choose a reason for hiding this comment

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

Should we remove the _get_work_folder function and refactor other code to just use get_workdir_from_session directly?
Especially since it's now just a single function call. I'd argue the same for the removal of _get_context_directory in switch_ui.py

There's also _get_work_folder() in another file that we could remove here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. I actually wasn't sure if the script is even used?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, it's old - probably unused in OpenPype - and it's been a while since we have used it too. We could consider it code debt for a future separate PR and discuss a more thorough cleanup.

@BigRoy
Copy link
Collaborator

BigRoy commented Mar 4, 2022

Separate from this PR we should look into what's up with this code.

That appears extremely connected to Maya only - and it seems to be part of a feature that I personally was unaware of. Is that used? Should that be removed/cleaned?

@iLLiCiTiT iLLiCiTiT requested a review from BigRoy March 4, 2022 10:28
@iLLiCiTiT
Copy link
Member Author

That appears extremely connected to Maya only - and it seems to be part of a feature that I personally was unaware of. Is that used? Should that be removed/cleaned?

It is extremely connected to maya and ftrack. I think it wasn't used for 3 years. Should be removed, but not in this PR.

@iLLiCiTiT iLLiCiTiT added type: refactor Structural changes not affecting functionality and removed type: enhancement Enhancements to existing functionality labels Mar 4, 2022
@mkolar mkolar removed the backend label Mar 4, 2022
@mkolar
Copy link
Member

mkolar commented Mar 4, 2022

Should that be removed/cleaned?

The whole tools should be removed and eventually re-written, this was an urgent production requirement a few years back when OP was used only in a handful of places. I'd leave it for a separate PR though

@iLLiCiTiT iLLiCiTiT merged commit ac3dd83 into develop Mar 4, 2022
@iLLiCiTiT iLLiCiTiT deleted the enhancement/OP-2844_Move-change-context-functions branch March 4, 2022 16:23
@mkolar mkolar added this to the 3.9.0 milestone Mar 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: refactor Structural changes not affecting functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants