-
Notifications
You must be signed in to change notification settings - Fork 0
Enhanced archive creation script and covered the edges cases. #95
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
base: development
Are you sure you want to change the base?
Conversation
|
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA). To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
| # Run archive(s) | ||
| for archive_type in archive_types: | ||
| result = archiver.archive(archive_type) | ||
| if not infra_constants.SUCCESS == result: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can the user see what what the error in archiver.archive? does archiver.archive log what was wrong? or should we log something from result?
lib/python/archive_infra.py
Outdated
| if space_admin_rc == 0: | ||
| __logger.info( | ||
| 'WLSDPLY-05027', | ||
| 'Admin have enough space to store all the archives. Managed hosts must have space for largest archive to transfer to admin.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be Admin has enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why this was marked as resolved. "have" still present instead of requested "has".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was working on the branch, going to push it now with the changes.
|
|
||
|
|
||
| def process_archives(nodes, model, model_context, machine_nodes, base_location, init_argument_map, on_prem_values, | ||
| space_status, log_file, wls_domain_name, per_host_space_key, archive_types, transfer_to_admin, do_upload): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow. This is a lot of arguments to a method. I think we should consider moving this into a class with class variables since I imagine a lot of these would be also required for other methods. I don't expect this to be done in this PR, but think about adding a ticket under epic JCS-15031, Refactor OCI WebLogic Migration tool code.
telake
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comments and the ones I see from Roberto are minor. Approving based on the assumption that you will be addressing those comments.
Case 1: Admin has space to store the largest archive
The log file also includes debugging statements which is not included in the MR.
Case 1 : migration_script.log
Case 2 :
log1:
migration_script.log
log2:
migration_script.log
Case 3.b : skip_transfer = true and Admin doesn’t have enough space to store all the archives but its own archives, then all nodes stores their respective archives including the admin.
Logs:
migration_script.log 3.b
Case 3.c : skip_transfer = true and Admin doesn’t have enough space to store its own archive, then all nodes stores their respective archives and TODO messages is printed for the nodes which doesn't have enough space.
logs:
migration_script.log
I tried to re-create the case 3.a, where "Admin have sufficient storage to store al the archives of each node" , but couldn't , as admin have very less storage.