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

DYN-6364 Skip Lucene indexing process in UIless mode #14523

Merged
merged 2 commits into from
Oct 26, 2023

Conversation

QilongTang
Copy link
Contributor

@QilongTang QilongTang commented Oct 25, 2023

Purpose

Skip the node indexing process when DynamoModel launches in UIless mode (usually happens in Player context), so there will be no index files created or locked. I believe this one line fix will work because Lucene Initialization, e.g. indexwriter creation is part of DynamoModel initialization. As a result, as long as indexwriter is not created, any indexing process later will not work and will not create any index and lock them or overwrite the existing index.

Notice this is a quick fix for 2.19.4, for future 2.19.x hotfixes, we may want to adopt the Lucene index improvements from master branch.

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Release Notes

Dynamo and Alias Dynamo Player can be launched at the same time

Reviewers

@DynamoDS/dynamo

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

@reddyashish
Copy link
Contributor

Did you get a chance to test this in Revit?

@github-actions
Copy link

github-actions bot commented Oct 25, 2023

⚠️ [run-bin-diff] - Files Added/Deleted::4 file(s) have been deleted!
⚠️ [run-bin-diff-net60-windows] - Files Added/Deleted::4 file(s) have been deleted!
(Updated: 2023-10-25-22:38:28)

@@ -60,6 +60,8 @@ internal LuceneSearchUtility(DynamoModel model)
/// </summary>
internal void InitializeLuceneConfig(string dirName, LuceneStorage storageType = LuceneStorage.FILE_SYSTEM)
{
if (DynamoModel.IsHeadless) return;
Copy link
Contributor

@BogdanZavu BogdanZavu Oct 25, 2023

Choose a reason for hiding this comment

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

So this should be set from all CLIs and D4R ( if not taken care of in core ) when the model starts without UI right ?
Also we should check with D4C3D.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so

@QilongTang
Copy link
Contributor Author

Did you get a chance to test this in Revit?

The bug is not reproducible in Revit because Revit Dynamo Player always try to reuse the same DynamoModel when launching Dynamo, in that case, it will be the same indexwriter so search works there. In Alias case, the Alias Dynamo Player and Dynamo are launched as two different processes,

@QilongTang
Copy link
Contributor Author

@reddyashish Also I think this PR will replace #14440 so we do not have to touch the startConfig, what do you think?

@QilongTang QilongTang added this to the 2.19.4 milestone Oct 25, 2023
@reddyashish
Copy link
Contributor

yeah wouldn't need it. Closed that PR

@@ -60,6 +60,8 @@ internal LuceneSearchUtility(DynamoModel model)
/// </summary>
internal void InitializeLuceneConfig(string dirName, LuceneStorage storageType = LuceneStorage.FILE_SYSTEM)
{
if (DynamoModel.IsHeadless) return;
Copy link
Member

Choose a reason for hiding this comment

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

perhaps you should mention this change in the comments for IsHeadLess mode, or do you not intend to port this change to master?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can update comments there

@QilongTang
Copy link
Contributor Author

The reported regression DynamoCoreWpfTests.CoreUserInterfaceTests.WorkspaceContextMenu_TestIfOpenOnRightClick is sporadic and passed locally.

@QilongTang QilongTang merged commit f3e97b0 into RC2.19.4_master Oct 26, 2023
32 checks passed
@QilongTang QilongTang deleted the SkipIndexUIlessMode branch October 26, 2023 07:26
QilongTang added a commit that referenced this pull request Mar 1, 2024
* skip Lucene indexing process in UIless mode

* Update comments
@QilongTang QilongTang mentioned this pull request Mar 1, 2024
9 tasks
mjkkirschner pushed a commit to mjkkirschner/Dynamo that referenced this pull request Mar 1, 2024
* skip Lucene indexing process in UIless mode

* Update comments
mjkkirschner added a commit that referenced this pull request Mar 1, 2024
* skip Lucene indexing process in UIless mode

* Update comments

Co-authored-by: Aaron (Qilong) <[email protected]>
QilongTang added a commit that referenced this pull request Mar 1, 2024
* Re-work watch node display logic (#14841)

* improvements (#14497)

* [DYN-6455] Add warning text to selection node's initial warning (#14833)

* Fix PostDiff job

* Update Selection.cs

* made the warning persistent and then add removal code

* [DYN-6585] Fix Export workspace as Image option from Menu Item (#14809)

* Fix PostDiff job

* Update DynamoView.xaml

* Revert UI Blocking Function calls (#14766)

* [DYN-6354] Fix incorrect confidence score display in ML suggestions (#14829)

* Fix PostDiff job

* Update NodeAutoCompleteSearchViewModel.cs

* Update

* DYN-5745 re enable copy/paste capabilities for library searchBar (#14492)

* feat(library): keydown events binding and copy/paste for clipboard management

* refactor(library): OnPasteFromClipboard function for clipboard management including comments

* DYN-6364 Skip Lucene indexing process in UIless mode (#14523)

* skip Lucene indexing process in UIless mode

* Update comments

---------

Co-authored-by: Ashish Aggarwal <[email protected]>
Co-authored-by: Enzo Batista <[email protected]>
QilongTang added a commit that referenced this pull request Mar 4, 2024
* skip Lucene indexing process in UIless mode

* Update comments
QilongTang added a commit that referenced this pull request Mar 4, 2024
* skip Lucene indexing process in UIless mode

* Update comments
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.

5 participants