-
Notifications
You must be signed in to change notification settings - Fork 49
feat: beta features access #4153
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
base: main
Are you sure you want to change the base?
Conversation
I think we should add this to our docs? In user-guide I mean? |
def __dir__(self): | ||
if self._fluent_connection is None: | ||
return ["is_active"] | ||
if self._app_utilities.is_beta_enabled(): |
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.
@prmukherj I believe that we should implement _is_beta_enabled()
at base-class level.
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.
yes, sounds right.
@@ -238,3 +237,15 @@ def transfer_mesh_to_solvers( | |||
clean_up_mesh_file, | |||
overwrite_previous, | |||
) | |||
|
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.
@prmukherj I believe that this should be a base class method that looks like:
def enable_beta_features(self):
"""Enable access to Fluent beta-features"""
# - app utilities encapsulates checking of current status
# - server figures out how to concretely enable beta (given that the action might be different per Fluent run mode)
self._app_utilities.enable_beta(self)
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.
AppUtilities should contain a enable_beta?
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.
I'm convinced of it.
Anything like a concrete TUI/Scheme dependency needs to be pushed as far into the back-end as possible — otherwise we'll slide back to where we were. For older servers, we can implement it in the app utilities client wrapper.
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.
Right, will do it. It's actually agrees with something suggested by Mainak as well. @mkundu1
if self._fluent_connection is None: | ||
return ["is_active"] |
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.
@prmukherj The strategy of what to do when detached is a core decision, not for concrete leaf classes.
Allow enabling of beta features both in meshing and solver mode.
For now only 2 features are there:
Both of these are accessible only after enabling beta features. Please refer tests for details.