feat: 15.3.0 #375
feat: 15.3.0 #375
Conversation
feat: detect network interference between SODA and server and SODA and pennsieve
…e in aftersign script
feat: one folder build option for packaging SODA
|
Thank you for submitting this pull request! We appreciate your contribution to the project. Before we can merge it, we need to review the changes you've made to ensure they align with our code standards and meet the requirements of the project. We'll get back to you as soon as we can with feedback. Thanks again! |
Reviewer's Guide by SourceryThis PR implements several key features to improve file handling and network connectivity detection. The main changes include a new system to detect and handle non-existent files in the dataset structure, empty folder cleanup, and network connectivity/firewall detection mechanisms. Sequence diagram for handling non-existent files in dataset structuresequenceDiagram
participant User
participant App
participant FileSystem
User->>App: Initiate dataset structure processing
App->>FileSystem: Check existence of files
FileSystem-->>App: Return file existence status
alt File does not exist
App->>User: Notify about non-existent files
App->>App: Remove non-existent file references
end
Sequence diagram for server live status check and handlingsequenceDiagram
participant App
participant Server
participant User
App->>Server: Check server live status
Server-->>App: Return live status
alt Server is not live
App->>User: Notify potential network issues
App->>App: Attempt to restart server
end
Class diagram for new firewall check moduleclassDiagram
class CheckFirewall {
+clientBlockedByExternalFirewall(url: string) bool
+blockedMessage: string
+hostFirewallMessage: string
}
class Axios
CheckFirewall --> Axios : uses
class ErrorHandler
CheckFirewall --> ErrorHandler : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Thanks for making updates to your pull request. Our team will take a look and provide feedback as soon as possible. Please wait for any GitHub Actions to complete before editing your pull request. If you have any additional questions or concerns, feel free to let us know. Thank you for your contributions! |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @aaronm-2112 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please remove debug console.log statements before merging (e.g. in renderer.js and globals.js)
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -331,12 +336,27 @@ const createPyProc = async () => { | |||
| pyflaskProcess.stderr.on("data", (data) => { | |||
| const logOutput = `[pyflaskProcess stderr] ${data.toString()}`; | |||
| sessionServerOutput += `${logOutput}`; | |||
| global.serverLive = false; | |||
There was a problem hiding this comment.
suggestion: Consolidate server status management into a single function
The serverLive status is being set to false in multiple places. Consider creating a dedicated function to manage server state changes to improve maintainability and consistency.
setServerStatus(false);| @@ -5416,6 +5416,131 @@ window.openPage = async (targetPageID) => { | |||
| } | |||
| } | |||
|
|
|||
| const purgeNonExistentFilesFromDatasetStructure = async (datasetStructure) => { | |||
There was a problem hiding this comment.
issue (complexity): Consider consolidating the recursive tree traversals into a single pass operation.
The current implementation performs three separate recursive tree traversals that could be reduced to two while maintaining the same functionality. Here's how to simplify it:
const handleNonExistentFiles = async (currentStructure, currentPath = "") => {
const nonExistentFiles = [];
const files = currentStructure?.files || {};
// Handle files at current level
for (const fileName in files) {
const fileData = files[fileName];
if (fileData.type === "local" && !window.fs.existsSync(fileData.path)) {
nonExistentFiles.push(`${currentPath}${fileName}`);
delete files[fileName];
log.info(`Deleting reference to non-existent file: ${currentPath}${fileName}`);
}
}
// Recurse into folders
const folders = currentStructure?.folders || {};
for (const folderName in folders) {
const subResults = await handleNonExistentFiles(
folders[folderName],
`${currentPath}${folderName}/`
);
nonExistentFiles.push(...subResults);
}
return nonExistentFiles;
};This combines the file collection and deletion into a single pass, reducing complexity while maintaining the same functionality. The folder cleanup can remain as a separate pass since it needs to run after all file operations are complete.
| const status = await ipcRenderer.invoke("get-server-live-status"); | ||
| return status; |
There was a problem hiding this comment.
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable)
| const status = await ipcRenderer.invoke("get-server-live-status"); | |
| return status; | |
| return await ipcRenderer.invoke("get-server-live-status"); | |
Explanation
Something that we often see in people's code is assigning to a result variableand then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
| const folderObjIsEmpty = (folderStructure) => { | ||
| // Check if there are no files at this level | ||
| const hasFiles = folderStructure.files && Object.keys(folderStructure.files).length > 0; | ||
| if (hasFiles) return false; |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (hasFiles) return false; | |
| if (hasFiles) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
|
|
Thanks for closing this pull request! If you have any further questions, please feel free to open a new issue. We are always happy to help! |
Summary by Sourcery
Introduce features to manage dataset structure by purging non-existent files and deleting empty folders. Enhance server status tracking and user feedback during initialization. Implement firewall checks to detect network issues. Update build scripts for folder-based Python builds and adjust CI workflows to include a new branch for folder builds.
New Features:
Enhancements:
Build:
CI: