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

python,poweshell,go example scripts for replication targets on DV #53

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sdunakhe
Copy link

python,poweshell,go example scripts for replication targets on DiskVolume (msdp or cc)

Copy link
Collaborator

@rkennedy rkennedy left a comment

Choose a reason for hiding this comment

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

Things could use a bit of cleanup.

A few syntax errors indicate that much of this code has never actually been run. As much as possible, let's try to provide working examples. Syntax errors don't inspire confidence that anything works.

Comment on lines -44 to +45
port = "1556"
storageUri = "storage/"
port = "1556"
storageUri = "storage/"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file now uses a mix of tabs and spaces for indentation. Can you please fix it to be consistent? I thought gofmt was supposed to make this kind of comment unnecessary.

"type": "diskPool",
"attributes": map[string]interface{}{
"name":dpName,
"diskVolumes":map[string]interface{}[,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comma looks like a syntax error. Am I mistaken?

print("--------------------------------------------------------\n");
print("-- This script requires Perl 5.20.2 or later --\n");
print("--------------------------------------------------------\n");
print("Executing this library requires some additional libraries like \n\t'LWP' \n\t'JSON'\ \n\t'Getopt'\ \n\n");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This message is pointless. The use statements above already enforce this, so nobody running this script will ever see the message printed here unless they already have the required modules, in which case they don't need to be told again.


sub print_disclaimer {
print("--------------------------------------------------------\n");
print("-- This script requires Perl 5.20.2 or later --\n");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think require v5.20.2 is a more concise way of stating this.

Comment on lines +68 to +69
print("Please provide the value for 'nbmaster'");
exit;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This and all the other error messages lack a trailing newline. Since you're requiring version 5.20, you may as well use say instead of print. Better yet, use die, which will print to stderr and exit non-zero for you.

Suggested change
print("Please provide the value for 'nbmaster'");
exit;
die "Please provide the value for 'nbmaster'\n;


my @dvid_split = @split /:/, $dvid
my $dvid_hex = sprintf "0x%02x", hex $dvid_hex[0];
my $url = "$PROTOCOL$master_server:$NB_PORT/netbackup/storage/storage-servers/".$stsid."/disk-volumes/".$dvid_hex."/replication-targets";
Copy link
Collaborator

Choose a reason for hiding this comment

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

String interpolation is used for the first few variables, but not for the latter ones. Why the change?

Comment on lines +84 to +90
- perl get_all_replication_targets_on_dv.pl -nbmaster <master_server> -username <username> -password <password> -stsid <storage server id> -dvid <diskvolume id> -payload <payload file path> [-domainname <domain_name>] [-domaintype <domain_type>];

Get replication target on diskvolume with specific replication target id
- perl get_replication_target_by_id_on_dv.pl -nbmaster <master_server> -username <username> -password <password> -stsid <storage server id> -dvid <diskvolume id> -payload <payload file path> [-domainname <domain_name>] [-domaintype <domain_type>];

Delete replication target on diskvolume
- perl post_delete_replication_target_on_dv.pl -nbmaster <master_server> -username <username> -password <password> -stsid <storage server id> -dvid <diskvolume id> -payload <payload file path> [-domainname <domain_name>] [-domaintype <domain_type>];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lose the semicolons at the ends of the lines.

Comment on lines +14 to +18
[string]$nbmaster = $(throw "Please specify the name of NetBackup master server using -nbmaster parameter."),
[string]$username = $(throw "Please specify the user name using -username parameter."),
[string]$password = $(throw "Please specify the password using -password parameter.")
[string]$stsid = $(throw "Please specify the storage server identifier using -stsid parameter.")
[string]$dvid = $(throw "Please specify the diskvolume id using -dvid parameter.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some lines end with commas and some don't. Is there significance to that?

if ([Net.ServicePointManager]::SecurityProtocol -notcontains 'Tls12') {
[Net.ServicePointManager]::SecurityProtocol += [Net.SecurityProtocolType]::Tls12
}
}G
Copy link
Collaborator

Choose a reason for hiding this comment

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

This G looks out of place.

Comment on lines +375 to +376
$del_rep_tgt_json = '{
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't valid JSON. The first brace introduces an object, so the second brace needs an attribute name before it.

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.

2 participants