DYN-6409 [Approach 2]: Disable PM requests with no-network mode#16238
Merged
aparajit-pratap merged 22 commits intoDynamoDS:masterfrom May 29, 2025
Merged
DYN-6409 [Approach 2]: Disable PM requests with no-network mode#16238aparajit-pratap merged 22 commits intoDynamoDS:masterfrom
aparajit-pratap merged 22 commits intoDynamoDS:masterfrom
Conversation
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-6409
Merged
4 tasks
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Contributor
|
I would vote for this approach, looks and works better IMO |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements an offline (no-network) mode by propagating a NoNetworkMode flag through sessions, HTTP clients, core nodes, and UI view models to disable network operations when Dynamo is offline.
- Introduce a
NoNetworkModesession parameter and a helper/HTTP handler to intercept and block outgoing requests. - Inject the custom handler into the PackageManager client and pass the offline flag to disable PM operations.
- Update core nodes and view models to guard against network calls and disable related UI commands.
Reviewed Changes
Copilot reviewed 23 out of 33 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/NodeServices/ExecutionSession.cs | Add NoNetworkMode key, ThrowIfNoNetworkMode helper, and a NoNetworkModeHandler |
| src/Libraries/CoreNodes/Web.cs | Guard WebRequestByUrl with ThrowIfNoNetworkMode |
| src/Libraries/CoreNodeModels/WebRequest.cs | Refactor NodeDescription attribute to use nameof |
| src/DynamoPackages/PackageManagerExtension.cs | Inject HttpClient with NoNetworkModeHandler, pass offline flag |
| src/DynamoPackages/PackageManagerClient.cs | Store NoNetworkMode, update constructor, clean up field usage |
| src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerClientViewModel.cs | Disable publish commands when offline |
| src/DynamoCoreWpf/ViewModels/Menu/PreferencesViewModel.cs | Expose NoNetworkMode property |
| src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs | Disable package manager menu commands when offline |
| src/DynamoCore/Models/DynamoModel.cs | Update documentation for NoNetworkMode |
| src/DynamoCore/Configuration/ExecutionSession.cs | Initialize NoNetworkMode in session parameters |
Files not reviewed (10)
- src/DynamoCore/DynamoCore.csproj: Language not supported
- src/DynamoCoreWpf/DynamoCoreWpf.csproj: Language not supported
- src/DynamoCoreWpf/Views/Menu/PreferencesView.xaml: Language not supported
- src/DynamoMLDataPipeline/DynamoMLDataPipeline.csproj: Language not supported
- src/DynamoPackages/DynamoPackages.csproj: Language not supported
- src/DynamoPackages/Properties/Resources.Designer.cs: Language not supported
- src/DynamoPackages/Properties/Resources.en-US.resx: Language not supported
- src/DynamoPackages/Properties/Resources.resx: Language not supported
- src/NodeServices/DynamoServices.csproj: Language not supported
- src/NodeServices/Properties/Resources.Designer.cs: Language not supported
Comments suppressed due to low confidence (5)
src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerClientViewModel.cs:455
- The previous check for
Selection.Count > 0was removed, causing this to return true when the selection is empty (sinceAllis vacuously true). Reintroduce the count check to prevent publishing with no selected nodes.
return DynamoSelection.Instance.Selection.All(x => x is Function) && AuthenticationManager.HasAuthProvider && !Model.NoNetworkMode;
src/NodeServices/ExecutionSession.cs:2
- [nitpick] Remove the unused
System.Collections.Genericusing directive to reduce clutter.
using System.Collections.Generic;
src/Libraries/CoreNodes/Web.cs:16
- Add a unit test to verify that
WebRequestByUrlthrows the expectedInvalidOperationExceptionwith the offline warning whenNoNetworkModeis enabled.
ExecutionSessionHelper.ThrowIfNoNetworkMode();
src/NodeServices/ExecutionSession.cs:132
- Add a unit test for
NoNetworkModeHandlerto confirm thatSendAsyncreturns HTTP 503 with the expected JSON payload and reason phrase.
var json = "{\"success\":false,\"message\":\"Application is in offline mode\",\"content\":null}";
src/Libraries/CoreNodes/Web.cs:13
- Add XML documentation to indicate that this method throws
InvalidOperationExceptionwhen running in offline mode viaThrowIfNoNetworkMode().
public static string WebRequestByUrl(string url)
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This approach is similar to #16191, except for initializing a custom HTTP client that returns a
service unavailableresponse and injecting that into the PM client for all requests.This depends on the version of PM client that has the changes in DynamoDS/PackageManagerClient#116.
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
(FILL ME IN) Brief description of the fix / enhancement. Use N/A to indicate that the changes in this pull request do not apply to Release Notes. Mandatory section
Reviewers
(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)
(FILL ME IN, optional) Any additional notes to reviewers or testers.
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of