Admin tool enhancements #159

Merged
merged 9 commits into from Feb 13, 2014

Conversation

Projects
None yet
3 participants
@suletm

suletm commented Feb 6, 2014

  • created a new branch for admin tool enhacements
  • added interactive support for set-metadata (stores and cluster types only for now)
  • added --auto argument for those who want to skip interactive prompts.
@vinothchandar

This comment has been minimized.

Show comment
Hide comment
@vinothchandar

vinothchandar Feb 6, 2014

Possibly rename this to "auto-update-metadata" to be more descriptive?

Possibly rename this to "auto-update-metadata" to be more descriptive?

This comment has been minimized.

Show comment
Hide comment
@suletm

suletm Feb 6, 2014

Owner

I wanted to make this a general argument that can be used for any other critical operation, not only for auto-update-metadata. Basically, this argument will make sure that ALL interactive prompts are disabled (useful for scripting).

In future, anyone who wants to add interactive prompts and/or batch mode to their operation, this argument can be used.

Does it make sense to you now ?

Owner

suletm replied Feb 6, 2014

I wanted to make this a general argument that can be used for any other critical operation, not only for auto-update-metadata. Basically, this argument will make sure that ALL interactive prompts are disabled (useful for scripting).

In future, anyone who wants to add interactive prompts and/or batch mode to their operation, this argument can be used.

Does it make sense to you now ?

This comment has been minimized.

Show comment
Hide comment
@vinothchandar

vinothchandar Feb 6, 2014

yes , in the case, can you leave a comment describing the intent to support more operations around this in the future?

yes , in the case, can you leave a comment describing the intent to support more operations around this in the future?

This comment has been minimized.

Show comment
Hide comment
@suletm

suletm Feb 7, 2014

Owner

Added a comment to the newest pull request

Owner

suletm replied Feb 7, 2014

Added a comment to the newest pull request

@vinothchandar

This comment has been minimized.

Show comment
Hide comment
@vinothchandar

vinothchandar Feb 6, 2014

to improve readability, do you want to switch the order of the if and else blocks.. ?

 if (auto) { 
    then execute... 
} else { 
    if(metadataUpdateSummary(...) 
}

to improve readability, do you want to switch the order of the if and else blocks.. ?

 if (auto) { 
    then execute... 
} else { 
    if(metadataUpdateSummary(...) 
}

This comment has been minimized.

Show comment
Hide comment
@suletm

suletm Feb 6, 2014

Owner

sure

Owner

suletm replied Feb 6, 2014

sure

This comment has been minimized.

Show comment
Hide comment
@suletm

suletm Feb 7, 2014

Owner

fixed in the latest pull request

Owner

suletm replied Feb 7, 2014

fixed in the latest pull request

@vinothchandar

This comment has been minimized.

Show comment
Hide comment
@vinothchandar

vinothchandar Feb 6, 2014

Can you rename this method to something like 'confirmMetadataUpdate' to improve readability?

Can you rename this method to something like 'confirmMetadataUpdate' to improve readability?

This comment has been minimized.

Show comment
Hide comment
@suletm

suletm Feb 6, 2014

Owner

yeah, makes sense

Owner

suletm replied Feb 6, 2014

yeah, makes sense

This comment has been minimized.

Show comment
Hide comment
@suletm

suletm Feb 7, 2014

Owner

fixed in the latest pull request

Owner

suletm replied Feb 7, 2014

fixed in the latest pull request

@vinothchandar

This comment has been minimized.

Show comment
Hide comment
@vinothchandar

vinothchandar Feb 12, 2014

Collaborator

Tofig, any updates on this?

Collaborator

vinothchandar commented Feb 12, 2014

Tofig, any updates on this?

@singhsiddharth

This comment has been minimized.

Show comment
Hide comment
@singhsiddharth

singhsiddharth Feb 12, 2014

Collaborator

Chinmay had added a method which had introduced a bug in Tofig's code. He fixed it and was running Hudson job to make sure everything else is ok.

Collaborator

singhsiddharth commented Feb 12, 2014

Chinmay had added a method which had introduced a bug in Tofig's code. He fixed it and was running Hudson job to make sure everything else is ok.

vinothchandar added a commit that referenced this pull request Feb 13, 2014

@vinothchandar vinothchandar merged commit 804249d into voldemort:master Feb 13, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment