feat: add command line URL queue #414
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe recent update to the Drifty CLI application enhances its functionality by allowing users to manage URLs through a command-line interface. This update enables users to add, list, remove URLs from a download queue, and perform batch downloads, leveraging a YAML file for efficient data storage and retrieval. Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
github-actions
bot
added
the
App 💻
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Yeah! You did it 🎉 Now, Relax 😉, Grab a drink ☕, and wait for the maintainers to check your contributions. Meanwhile, you can discuss on other issues and solve them 😀. Thank You 😃!
Meanwhile you can also discuss about the project in our Discord Server 😀
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- CLI/src/main/java/cli/support/Constants.java (1 hunks)
- CLI/src/main/java/main/Drifty_CLI.java (4 hunks)
Additional comments: 10
CLI/src/main/java/cli/support/Constants.java (1)
- 9-12: The addition of the new string constants
ADD_FLAG,REMOVE_FLAG,LIST_FLAG, andGET_FLAGis consistent with the PR objectives to support new CLI commands.CLI/src/main/java/main/Drifty_CLI.java (9)
- 43-43: The new constant
URL_LIST_FILEhas been correctly defined to specify the file used for storing the URL queue.- 65-86: The
ADD_FLAGcase in themainmethod correctly checks for a valid URL, adds it to the file, and then lists the URLs.- 87-90: The
LIST_FLAGcase in themainmethod correctly triggers the listing of URLs.- 91-105: The
REMOVE_FLAGcase in themainmethod correctly parses the line number and triggers the removal of the corresponding URL.- 106-111: The
GET_FLAGcase in themainmethod correctly triggers the batch download process.- 240-259: The
listUrlsmethod correctly reads the URL list from the file and prints it to the console.- 261-284: The
removeUrlmethod correctly removes a URL from the list based on the provided index.- 287-293: The
addUrlToFilemethod correctly appends a new URL to the end of the URL list file.- 296-361: The
batchTextDownloadermethod correctly reads URLs from the file and processes them for downloading.
|
@Femi-lawal Awesome work 👏. Some minor changes are required. Will let you know tomorrow 😁. |
❌ Linting errors found!@Femi-lawal Please fix the following errors: CLI/src/main/java/main/Drifty_CLI.java:80:30: Control variable 'i' is modified. [ModifiedControlVariable]
CLI/src/main/java/main/Drifty_CLI.java:96:34: Control variable 'i' is modified. [ModifiedControlVariable]
CLI/src/main/java/main/Drifty_CLI.java:80:30: Control variable 'i' is modified. [ModifiedControlVariable]
CLI/src/main/java/main/Drifty_CLI.java:96:34: Control variable 'i' is modified. [ModifiedControlVariable] |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- CLI/src/main/java/main/Drifty_CLI.java (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- CLI/src/main/java/main/Drifty_CLI.java
There was a problem hiding this comment.
@Femi-lawal Please make the following changes.
I am sorry for the delay in reviewing your PR. I was very busy the last couple of days.
Appreciate your patience 😁 👏.
If you need any further information, please let me know.
I've made the change |
|
@Femi-lawal Sorry for the delay in reviewing this PR. I was busy the last week. I will review this now. |
There was a problem hiding this comment.
@Femi-lawal Please make the three requested changes along with the below mentioned issue. Then, we can have this PR merged 🎉!
- If only 1 url exist, the print message does not sound good as shown below 👇. Can you please fix this? Something like this 👇 can be added.
There's only one URL in the list! Would you like to remove it? (Y/N) :
|
@Femi-lawal Please make the requested changes 😁. |
@SaptarshiSarkar12 I think the amount of code required just to give a better experience for this niche example is unjustified The error handling for Unfortunately, too much time has passed, and I am very busy now. I can't carve out any more time to make significant code changes to the pull request and test them. |
|
@Femi-lawal It's fine. No problem. |
❌ Linting errors found!@Femi-lawal Please fix the following errors: CLI/src/main/java/main/Drifty_CLI.java:92:30: Control variable 'i' is modified. [ModifiedControlVariable]
CLI/src/main/java/main/Drifty_CLI.java:107:31: Control variable 'i' is modified. [ModifiedControlVariable]
CLI/src/main/java/main/Drifty_CLI.java:135:31: Control variable 'i' is modified. [ModifiedControlVariable]
CLI/src/main/java/main/Drifty_CLI.java:153:30: Control variable 'i' is modified. [ModifiedControlVariable]
CLI/src/main/java/main/Drifty_CLI.java:83:30: Control variable 'i' is modified. [ModifiedControlVariable] |
There was a problem hiding this comment.
Actionable comments posted: 8
Out of diff range and nitpick comments (1)
CLI/src/main/java/main/Drifty_CLI.java (1)
56-57: The use of a constant for the YAML filename is good for maintainability. However, consider externalizing such constants to a configuration file or environment variables for easier management across different environments.
…re/resolve-conflicts
…at and added help menu for each of them
There was a problem hiding this comment.
Actionable comments posted: 3
Out of diff range and nitpick comments (1)
CLI/src/main/java/main/Drifty_CLI.java (1)
Line range hint
341-456: Handle potential YAML parsing exceptions more robustly.+ try { Yaml yamlParser = Utility.getYamlParser(); messageBroker.msgLogInfo("Trying to load YAML data file (" + batchDownloadingFile + ") ..."); try (FileInputStream yamlInputStream = new FileInputStream(batchDownloadingFile); InputStreamReader yamlDataFile = new InputStreamReader(yamlInputStream)) { Map<String, List<String>> data = yamlParser.load(yamlDataFile); messageBroker.msgLogInfo("YAML data file (" + batchDownloadingFile + ") loaded successfully"); // Additional logic here... } catch (IOException e) { messageBroker.msgDownloadError("Failed to load YAML data file (" + batchDownloadingFile + ") ! " + e.getMessage()); } catch (YAMLException e) { String errorMessage = e.getMessage(); if (errorMessage.contains("duplicate key")) { messageBroker.msgBatchError("Duplicate keys are not allowed in the YAML file!"); } else if (errorMessage.contains("recursive key")) { messageBroker.msgBatchError("Recursive keys are not allowed in the YAML file!"); } else { messageBroker.msgBatchError("An unknown error occurred while parsing the YAML file! " + errorMessage); } } + } catch (Exception e) { + messageBroker.msgBatchError("An unexpected error occurred while processing the YAML file: " + e.getMessage()); + Environment.terminate(1); + }This change adds a general exception handler to catch any unexpected exceptions that may occur during the YAML file processing, ensuring that all types of errors are handled gracefully.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- .github/linters/sun_checks.xml (1 hunks)
- CLI/src/main/java/cli/support/Constants.java (2 hunks)
- CLI/src/main/java/cli/utils/Utility.java (2 hunks)
- CLI/src/main/java/main/Drifty_CLI.java (12 hunks)
Files skipped from review due to trivial changes (1)
- .github/linters/sun_checks.xml
Additional Context Used
Learnings (2)
Common
User: SaptarshiSarkar12 PR: SaptarshiSarkar12/Drifty#414 File: CLI/src/main/java/main/Drifty_CLI.java:795-816 Timestamp: 2024-05-06T06:45:34.696Z Learning: Enhancement of the URL normalization logic in the `addUrlToFile` method of `Drifty_CLI.java` is planned to be addressed later as per user SaptarshiSarkar12's acknowledgment.CLI/src/main/java/main/Drifty_CLI.java (2)
User: SaptarshiSarkar12 PR: SaptarshiSarkar12/Drifty#414 File: CLI/src/main/java/main/Drifty_CLI.java:122-140 Timestamp: 2024-05-06T06:07:27.386Z Learning: SaptarshiSarkar12 prefers the current implementation of the `REMOVE_FLAG` case in `Drifty_CLI.java` for its clarity and readability, despite suggestions to reduce nesting.User: SaptarshiSarkar12 PR: SaptarshiSarkar12/Drifty#414 File: CLI/src/main/java/main/Drifty_CLI.java:795-816 Timestamp: 2024-05-06T06:45:34.696Z Learning: Enhancement of the URL normalization logic in the `addUrlToFile` method of `Drifty_CLI.java` is planned to be addressed later as per user SaptarshiSarkar12's acknowledgment.
Additional comments not posted (4)
CLI/src/main/java/cli/support/Constants.java (2)
9-12: Constants for CLI flags are well-defined and follow a consistent naming convention.
23-23: The confirmation message for removing all URLs is clear and user-friendly.CLI/src/main/java/cli/utils/Utility.java (2)
7-9: Imports for YAML processing are correctly added.
111-118: The methodgetYamlParseris well-implemented with appropriate settings to avoid common YAML parsing issues like duplicate keys and recursive keys.
…n only when links queue is used to avoid throwing errors everytime
There was a problem hiding this comment.
Actionable comments posted: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- CLI/src/main/java/main/Drifty_CLI.java (11 hunks)
Additional Context Used
Learnings (2)
Common
User: SaptarshiSarkar12 PR: SaptarshiSarkar12/Drifty#414 File: CLI/src/main/java/main/Drifty_CLI.java:795-816 Timestamp: 2024-05-06T06:45:34.696Z Learning: Enhancement of the URL normalization logic in the `addUrlToFile` method of `Drifty_CLI.java` is planned to be addressed later as per user SaptarshiSarkar12's acknowledgment.CLI/src/main/java/main/Drifty_CLI.java (1)
User: SaptarshiSarkar12 PR: SaptarshiSarkar12/Drifty#414 File: CLI/src/main/java/main/Drifty_CLI.java:795-816 Timestamp: 2024-05-06T06:45:34.696Z Learning: Enhancement of the URL normalization logic in the `addUrlToFile` method of `Drifty_CLI.java` is planned to be addressed later as per user SaptarshiSarkar12's acknowledgment.
Additional comments not posted (19)
CLI/src/main/java/main/Drifty_CLI.java (19)
12-12: Consider handling potential exceptions fromSafeConstructoras it might throw runtime exceptions if the YAML structure is incorrect.
71-74: Ensure proper error handling for missing filename argument.
80-84: Ensure proper error handling for missing download directory argument.
92-103: Ensure that theaddcommand correctly handles multiple URLs and provides appropriate feedback when no URLs are provided.
105-108: Ensure that thelistcommand correctly sets the YAML file path and lists URLs.
109-120: Ensure that thegetcommand handles batch downloading correctly, checks for empty YAML, and removes all URLs after processing.
121-140: Ensure that theremovecommand handles the removal of specific URLs or all URLs correctly and provides appropriate feedback when no line number is provided.
Line range hint
142-155: Ensure that the batch download functionality correctly handles file path validation and processes the YAML file correctly.
Line range hint
341-456: Review the batch downloading logic to ensure it handles YAML parsing errors and file I/O operations robustly.
617-644: Improve URL normalization logic to handle more edge cases.
647-670: Ensure that the YAML file existence check and creation logic are robust and provide clear feedback.
672-678: Ensure that the method correctly identifies empty YAML files.
680-704: Review the YAML data loading logic to ensure it handles file existence, reads data correctly, and manages YAML parsing errors.
706-738: Review the YAML data saving logic to ensure it handles file backup creation, data writing, and error recovery robustly.
740-756: Ensure that thelistUrlsmethod correctly loads data, checks for emptiness, and lists URLs robustly.
758-793: Review theremoveUrlmethod to ensure it handles URL removal based on indices correctly and saves the updated YAML data.
795-819: Review theremoveAllUrlsmethod to ensure it handles the deletion of all URLs from the YAML file robustly and provides appropriate feedback.
822-844: Review theaddUrlToFilemethod to ensure it validates URLs, normalizes them, checks for duplicates, and saves them correctly.
847-853: Ensure that the method for setting the YAML file path handles path validation and error feedback correctly.
There was a problem hiding this comment.
Actionable comments posted: 4
Out of diff range and nitpick comments (2)
CLI/src/main/java/main/Drifty_CLI.java (2)
Line range hint
339-454: Improve error handling in thebatchDownloadermethod to catch specific YAML parsing exceptions.- } catch (YAMLException e) { + } catch (YAMLException | IOException e) {Adding
IOExceptionto the catch block ensures that all potential I/O errors during YAML parsing are handled appropriately.
707-738: Ensure atomicity insaveYamlDatato prevent data loss.Consider implementing a transaction-like mechanism or using a temporary file during the write operation to ensure that data is not lost if an error occurs during the file write process. This approach helps in maintaining data integrity.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- CLI/src/main/java/main/Drifty_CLI.java (11 hunks)
Additional Context Used
Learnings (2)
Common
User: SaptarshiSarkar12 PR: SaptarshiSarkar12/Drifty#414 File: CLI/src/main/java/main/Drifty_CLI.java:795-816 Timestamp: 2024-05-06T06:45:34.696Z Learning: Enhancement of the URL normalization logic in the `addUrlToFile` method of `Drifty_CLI.java` is planned to be addressed later as per user SaptarshiSarkar12's acknowledgment.CLI/src/main/java/main/Drifty_CLI.java (1)
User: SaptarshiSarkar12 PR: SaptarshiSarkar12/Drifty#414 File: CLI/src/main/java/main/Drifty_CLI.java:795-816 Timestamp: 2024-05-06T06:45:34.696Z Learning: Enhancement of the URL normalization logic in the `addUrlToFile` method of `Drifty_CLI.java` is planned to be addressed later as per user SaptarshiSarkar12's acknowledgment.
Additional comments not posted (1)
CLI/src/main/java/main/Drifty_CLI.java (1)
110-120: Ensure proper error handling when the YAML file is empty.This script checks if the
isEmptyYamlmethod is called and handled properly in the scenario where the YAML file is empty, ensuring that the program exits gracefully with an appropriate error message.
There was a problem hiding this comment.
@Femi-lawal Looks good to merge 👍.
Thanks for contributing 🚀 🚀.
You may join our Discord server - https://discord.gg/DeT4jXPfkG to get updates about the project.
Fixes issue
Fixes #410
Changes proposed
addin the CLI to add URLs to a local text file for batch processing.listcommand to display all the URLs currently stored in the text file.removecommand to remove the URL at the specified line number.getcommand to start downloading the contents of the text file.Check List (Check all the applicable boxes)
Screenshots
Help:
Add:
Remove:
List:
Get:
Summary by CodeRabbit