-
Notifications
You must be signed in to change notification settings - Fork 70
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
Update java tools with methods to be called from online tool. #106
Conversation
…ess repetition of same function being called multiple times from python code Signed-off-by: rtgdk <[email protected]>
Signed-off-by: rtgdk <[email protected]>
… retval, instead return verify only Signed-off-by: rtgdk <[email protected]>
…ective conversion onlineFunction method so there is no need to call verifyRDFFile for each of the RDF file before passing it to them. * Removes repetition of code Signed-off-by: rtgdk <[email protected]>
…oSpreadsheet conversion onlineFunction method so there is no need to call verifyTagFile for each of the Tag Value file before passing it to them. * Removes repetition of code Signed-off-by: rtgdk <[email protected]>
I have added verification check for RDF files in all the RDF Conversion. Now there is no need to call PS : There is still some work left - Verification in |
@rtgdk I can see the issue with calling the main method directly as it is designed to be used as a command line utility and there are scripts which depend on the System.exit being called to properly set the exit code. Making a copy of the main method will cause some additional maintenance effort as we now have 2 copies of the code we need to update if there is a fix or enhancement. Rather than making a copy for the online tool access, I would recommend creating changing the main methods to call the onlineFunction and catch the exception. On catching the exception, the main method could System.print the exception message and System.exit(ERROR_CODE). |
@rtgdk Any System.exit calls will be within the tool utility class itself (e.g. TagToSpreadsheet.java). In most cases, they will just be in the main method. You should not find any System.exit calls for any methods called for any other classes. |
The only difference is the CompareSPDXDocs takes a File as an input and TagToRDF takes an input stream. The CompareSPDXDocs method calls the TagToRDF convertTagFileToRdf method after opening the file. The code is not well structured, the compareSPDXDocs convertTagFileToRdf should be moved to the TagToRDF file to keep all of the TagtoRdf methods together in one logical place. I do recall that the TagToRDF.convertTagFileToRdf was created so that tools and utilities could call the method directly without having to go through the main method (avoiding the System.exit problem). |
@rtgdk The Excel verification is done in the method |
…intaining copies of the same function Signed-off-by: rtgdk <[email protected]>
Thanks. I saw the flow of the Spreadsheet conversion code. No need for a separate function to validate Spreadsheet since they are only input in conversion and proper error are thrown to the online tool. I'll push the changes you suggested ( |
Good then we won't need SecurityManager for the onlineFunction. |
About that |
I have added methods somewhat similar to main methods of different tools but with fewer repetition of calling the same function from Python. Some of the methods returns warning/error message which can be displayed to the user. Also, I have removed any
System.exit
part in the method and rather thrown an OnlineToolException for every error that occur in the method. This can be caught by Jpype.This way the JVM won't shutdown causing Django app to not shutdown inappropriately, Most of the time OnlineToolException won't be called as file exist and file check happens in python only but still I have added
throw
to them.I hope this patch will ensure that System.exit is never encountered and the online app is run smoothly. If any of the internal methods are causing System.exit, I can later add
SecurityManager
to these methods and catchSecurityException
.