Skip to content

Conversation

mkundu1
Copy link
Contributor

@mkundu1 mkundu1 commented Nov 18, 2023

The cached state returned from DataModelCache.get_state(rules, obj, name_key) will contain display names or internal names based on the parameter name_key

@seanpearsonuk
Copy link
Collaborator

seanpearsonuk commented Nov 20, 2023

@prmukherj Have you tried this branch out? Is the cached state now coming with display names?

@seanpearsonuk, yes pulled it into my branch and tried it with switching display names on. Working fine.

@mkundu1
Copy link
Contributor Author

mkundu1 commented Nov 20, 2023

@prmukherj I'll investigate the test failures.

@mkundu1
Copy link
Contributor Author

mkundu1 commented Nov 21, 2023

Looking at one of the failing test tests/test_data_model_cache.py::test_get_cached_values_in_command_arguments, it seems it now correctly returns the value from the cache. When the cache contains the entry 'TaskObject:TaskObject1': {'TaskList': [], 'InactiveTaskList': [], '_name_': 'Import Geometry', 'Arguments': {'FileName': 'Bob'}, 'CommandName': 'ImportGeometry', 'TaskType': 'Simple', 'ObjectPath': '', 'State': 'Out-of-date'},, which is not changed and we query the state at path TaskObject:Import Geometry/Arguments, it now returns {'FileName': 'Bob'}. Earlier it used to return DataModelCache.Empty. This is probably making a difference that fails the test with these new changes.

@prmukherj
Copy link
Collaborator

Thank you @mkundu1 for the input. I'll check this.

mkundu1 and others added 15 commits November 22, 2023 10:10
* TaskObject updates.

* Minor issue fix in launcher.

* Fix return state.

* Minor fixes.

* Linearise logic for getting ip and port.

* Revert changes.

* Added test for callable task-objects.
@mkundu1
Copy link
Contributor Author

mkundu1 commented Nov 23, 2023

@prmukherj @seanpearsonuk
Among the remaining failing tests, one type of failure seems to be due to the code queries for TaskList and InactiveTaskList parameters which return internal names. They don't match with the state as the state is received with display names from the cache.
There is another type of failure which I've locally tested to fix by adding some sleep before the assert, e.g. the following assert in tests/test_meshing_workflow.py::test_extended_wrapper:

assert import_geometry.arguments.get_state(explicit_only=True) == dict(
    FileName=None
)

The assert is executed before the cache is updated with the streamed state.

@prmukherj
Copy link
Collaborator

@mkundu1, @seanpearsonuk , what should be our approach then? Shall we fix the test along this new behavior? The earlier behavior seems inaccurate then, right?

@mkundu1 mkundu1 merged commit 53aa027 into main Nov 23, 2023
@mkundu1 mkundu1 deleted the feat/cache branch November 23, 2023 22:51
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.

3 participants