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

Wdt 1112 appfileoverrides #1149

Merged
merged 11 commits into from
Jul 19, 2022
Merged

Wdt 1112 appfileoverrides #1149

merged 11 commits into from
Jul 19, 2022

Conversation

CarolynRountree
Copy link
Contributor

Fixes #1112

Handle application folder directories that contain app and plan directories.
Write directory to plan.xml config-root
make sure directory output is correct for option -remote

Copy link
Member

@rakillen rakillen left a comment

Choose a reason for hiding this comment

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

Minor changes

return app_folder

def _create_application_directory(self, application_name, application_dict):
_method_name = 'createAppFolder'
Copy link
Member

Choose a reason for hiding this comment

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

method name is _create_application_directory

@@ -671,11 +716,13 @@ public void extractApplication(String applicationPath, File domainHome) throws W
validateExistingDirectory(domainHome, "domainHome", getArchiveFileName(), METHOD);

String appPath = applicationPath;
if (!applicationPath.startsWith(ARCHIVE_APPS_TARGET_DIR)) {
if (!applicationPath.startsWith(ARCHIVE_APPS_FOLD_TARGET_DIR) &&
!applicationPath.startsWith(ARCHIVE_APPS_TARGET_DIR)) {
Copy link
Member

Choose a reason for hiding this comment

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

Not sure this level of intention is consistent with the rest of the code.

element = elements.item(0)
element.setNodeValue(plan_dir)
element.setTextContent(plan_dir)
# document.appendChild(element)
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the commented code

@@ -62,7 +62,7 @@ public class WLSDeployArchive {
* Top-level archive subdirectory where the applications are stored and the subdirectory to which
* they will be extracted. This is for structured applications found under /app
*/
public static final String ARCHIVE_APPS_FOLD_TARGET_DIR = WLSDPLY_ARCHIVE_BINARY_DIR + "/application_folder";
public static final String ARCHIVE_APPS_FOLD_TARGET_DIR = WLSDPLY_ARCHIVE_BINARY_DIR + "/applicationsFolder";
Copy link
Member

Choose a reason for hiding this comment

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

I hate to be a pain but please change this name to applicationFolders to more accurately reflect the purpose/content. That is, this is a location for structured application folders not a single folder for applications.

Copy link
Member

Choose a reason for hiding this comment

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

If you don't like that, we could change to structuredApplications instead...

Copy link
Member

Choose a reason for hiding this comment

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

I think structuredApplications is clearer for the user. Sounds more intuitive as to why we separated the two application types.

Copy link
Member

@robertpatrick robertpatrick left a comment

Choose a reason for hiding this comment

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

Other than the one rename, the rest looks fine. Thanks.

@sonarcloud
Copy link

sonarcloud bot commented Jul 19, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@ddsharpe ddsharpe merged commit 11b3973 into main Jul 19, 2022
@ddsharpe ddsharpe deleted the WDT-1112-appfileoverrides branch July 19, 2022 22:04
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.

RFE - generic file loading overrides
4 participants