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

General: Functions for current context #4324

Merged
merged 6 commits into from
Feb 1, 2023

Conversation

iLLiCiTiT
Copy link
Member

@iLLiCiTiT iLLiCiTiT commented Jan 16, 2023

Brief description

Defined more functions to receive current context information and added the methods to host integration so host can affect the result.

Description

This is one of steps to reduce usage of legacy_io.Session. This change define how to receive current context information -> call functions instead of accessing legacy_io.Session or os.environ directly. Plus, direct access on session or environments is unfortunatelly not enough for some DCCs where multiple workfiles can be opened at one time which can heavily affect the context but host integration sometimes can't affect that at all.

HostBase already had implemented get_current_context, that was enhanced by adding more specific methods get_current_project_name, get_current_asset_name and get_current_task_name. The same functions were added to ~/openpype/pipeline/cotext_tools.py. The functions in context tools are calling host integration methods (if are available) otherwise are using environent variables as default implementation does. Also was added get_current_host_name to receive host name from registered host if is available or from environment variable.

Additional info

Didn't change usages of legacy_io.Session anywhere. We should make a decision and agree on an approach and then start replace current usages. Anyway there must be backwards compatibility for some time.

Possible changes

  • Use legacy_io.Session instead of os.environ. But I wanted to avoid that as the goal is to reduce usage of legacy_io. Those functions are for "reading", "change" of current context in legacy_io is changing environemnts too (if does not then it should).
  • Add set_current_context function to replace change_current_context and some logic in workfiles tool.
  • The functionality is doubled in default implementation of host and in context tools (I didn't find out good or better solution).

Testing notes:

It's more about decisions. Any comments are welcome

@iLLiCiTiT iLLiCiTiT self-assigned this Jan 16, 2023
@iLLiCiTiT iLLiCiTiT added the type: enhancement Enhancements to existing functionality label Jan 16, 2023
Copy link
Member

@kalisp kalisp left a comment

Choose a reason for hiding this comment

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

Just nitpicking on condition format and missing docstring, other way it looks good.

(Maybe just one thing, shouldn't these be also in pipeline/__init__.py to highlight them a bit more?

openpype/pipeline/context_tools.py Outdated Show resolved Hide resolved
@iLLiCiTiT
Copy link
Member Author

Agree with both comment and added them...

@iLLiCiTiT iLLiCiTiT merged commit e250ed5 into develop Feb 1, 2023
@iLLiCiTiT iLLiCiTiT deleted the feature/functions_for_current_context branch February 1, 2023 11:39
@github-actions github-actions bot added this to the next-patch milestone Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Enhancements to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants