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

Feature atmos #1

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

Feature atmos #1

wants to merge 12 commits into from

Conversation

dchev
Copy link
Contributor

@dchev dchev commented May 20, 2019

atmos support to bucket-wipe.

@dchev dchev requested a review from twincitiesguy May 20, 2019 19:01
Copy link
Collaborator

@twincitiesguy twincitiesguy left a comment

Choose a reason for hiding this comment

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

bucket should only be required if not in Atmos mode
also, please add lots of comments and use very descriptive and accurate variable names to help people understand the graph structure and how deletes are processed

README.md Outdated
of subtenantid/uid, e.g.
640f9a5cc636423fbc748566b397d1e1/uid1

-atmos,--atmos the tool is used to delete Atmos namespace
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't have a short-option here (leave it null) in the CLI option

README.md Outdated
-e,--endpoint <URI> the endpoint to connect to, including
protocol, host, and port
protocol, host, and port or Atmos access
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't need the additional note here (endpoint is the right term in either S3 or Atmos)

README.md Outdated

prefix

-port,--atmosport <port> Atmos access point port (default 80)
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't need this (port is in the URI)

README.md Outdated
-port,--atmosport <port> Atmos access point port (default 80)

-s,--secret-key <secret-key> the secret key or Atmos Shared secret if
option -atmos is used
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't need additional note here (secret key is the right term)

@@ -41,6 +41,38 @@
import java.util.concurrent.LinkedBlockingDeque;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;
// Atmos imports
Copy link
Collaborator

Choose a reason for hiding this comment

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

need to organize imports (in IJ, Ctrl-Alt-O)

private static Options options() {
Options options = new Options();
options.addOption(Option.builder().longOpt("stacktrace").desc("displays full stack trace of errors").build());
options.addOption(Option.builder("h").longOpt("help").desc("displays this help text").build());
options.addOption(Option.builder("atmos").longOpt("atmos")
Copy link
Collaborator

Choose a reason for hiding this comment

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

leave the builder call empty (no short-option)

@@ -118,25 +176,32 @@ public void run() {
}

private static void printHelp() {
new HelpFormatter().printHelp("java -jar bucket-wipe.jar [options] <bucket-name>", options());
new HelpFormatter().printHelp("java -jar bucket-wipe.jar [options] <bucket-name or atmos directory path>", options());
Copy link
Collaborator

Choose a reason for hiding this comment

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

leave this the same (this will just be the bucket)

options.addOption(Option.builder("p").longOpt("prefix").hasArg().argName("prefix")
.desc("deletes only objects under the specified prefix").build());
options.addOption(Option.builder("o").longOpt("atmos-object-space").desc("atmos only: use atmos object space " +
Copy link
Collaborator

Choose a reason for hiding this comment

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

add in description of "p" option, " or Atmos namespace path"

@@ -163,38 +228,157 @@ private static Options options() {
private boolean hierarchical;
private String sourceListFile;

// *********Atmos related
private boolean atmosOption;
private String remoteroot; // remoteroot is Atmos' bucket name
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove this - namespace path will just be prefix

private long dirCount;
private long fileCount;
private SimpleDirectedGraph<TaskNode, DefaultEdge> graph;
private Set<TaskNode> failedItems;
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove this.. tracking failed items may run out of memory

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.

None yet

2 participants