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.

Sign up for GitHub

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

Jump to bottom

Problem: The Automation Tools AMClient script could be easier to jump into and add functionality #47

Closed
ross-spencer opened this issue Feb 21, 2018 · 2 comments
Closed

Problem: The Automation Tools AMClient script could be easier to jump into and add functionality #47

ross-spencer opened this issue Feb 21, 2018 · 2 comments
Assignees
@ross-spencer
Labels
enhancement

Comments

@ross-spencer
Copy link
Contributor

As a developer I like to be able to see a division in responsibilities of code. For example being able to see clearly where I can add new CLI args, or new logic.

The AMClient.py script in Automation Tools currently tries to do a lot of this in a single script - args/logging/functional logic. I think there is an opportunity to pull those three pieces apart to make it easier for developers to work with and open the opportunity for smaller refactors in the future.

AMClient.py is a good candidate to have this attention paid to it as its functionality is only likely to grow as it wraps the AM API capabilities in a consistent way.

Proposal:

  • Create a logging configuration script that can provide a model for sharing across the other Automation Tools (Transfer.py would be the first to make use of this)
  • Separate the CLI argument definitions into a script on its own so that it becomes an entry point for new functionality being added by developers.
  • Leave the core features of the script in place (API calls and handling) and import the new scripts at the module level.

This will reduce the size of each of the scripts and hopefully make it a bit easier for folk to delve into.
@ross-spencer ross-spencer self-assigned this Feb 21, 2018
@ross-spencer ross-spencer mentioned this issue Feb 21, 2018
Closed
@sevein
Copy link
Member

sevein commented Feb 21, 2018 •
edited
Loading

During your endeavor please try to keep in mind that this repo doesn't follow Python's best practices and people is going to expect that things keep working as usual, i.e. clone the repo and use it as it was described in the installation instructions, files like transfers/transfer.py are executables and this should probably keep being this way unless we decide to break the contract. Sounds like #46 doesn't really change it so that's great.

I think that we need a long-term plan that serves as basis for future efforts like #46. To be clear, I like #46 but I'm also wondering if the next effort should be previously discussed. I personally like the way that https://github.com/docker/compose is arranged and I think that it'd be awesome that automation-tools moves into that direction.

qubot pushed a commit that referenced this issue Feb 21, 2018
@ross-spencer
Refactors amclient.py
8603fbb 
* Creates new logging configuration script to reduce redundancy
* Separates responsibility of amclient, defaults, and cli args
* Cleans up argparse output using metavar
* Adds Conformity to PEP8 to amclient.py and transfers.py
* Returns user friendly string via getattr if function call fails in main
* Improved error return from JSON request function in amclient.py
* Adds .db and .lck files to .gitignore
* Works at module and executable level
* E402 flake8 warning ignored in Tox to enable module/exe to work
* Captures connection errors in transfers.py better
* Initial changes provides scope for future refactoring as required
* Addresses #47
qubot pushed a commit that referenced this issue Feb 21, 2018
@ross-spencer
Refactors amclient.py
1cef7ce 
* Creates new logging configuration script to reduce redundancy
* Separates responsibility of amclient, defaults, and cli args
* Cleans up argparse output using metavar
* Adds Conformity to PEP8 to amclient.py and transfers.py
* Returns user friendly string via getattr if function call fails in main
* Improved error return from JSON request function in amclient.py
* Adds .db and .lck files to .gitignore
* Works at module and executable level
* E402 flake8 warning ignored in Tox to enable module/exe to work
* Captures connection errors in transfers.py better
* Initial changes provides scope for future refactoring as required
* Addresses #47
@ross-spencer ross-spencer mentioned this issue Feb 21, 2018
Merged
qubot pushed a commit that referenced this issue Feb 21, 2018
@ross-spencer
Refactors amclient.py
80d320b 
* Creates new logging configuration script to reduce redundancy
* Separates responsibility of amclient, defaults, and cli args
* Cleans up argparse output using metavar
* Adds Conformity to PEP8 to amclient.py and transfers.py
* Returns user friendly string via getattr if function call fails in main
* Improved error return from JSON request function in amclient.py
* Adds .db and .lck files to .gitignore
* Works at module and executable level
* E402 flake8 warning ignored in Tox to enable module/exe to work
* Captures connection errors in transfers.py better
* Initial changes provides scope for future refactoring as required
* Addresses #47
qubot pushed a commit that referenced this issue Feb 22, 2018
@ross-spencer
Refactors amclient.py
451ca2f 
* Creates new logging configuration script to reduce redundancy
* Separates responsibility of amclient, defaults, and cli args
* Cleans up argparse output using metavar
* Adds Conformity to PEP8 to amclient.py and transfers.py
* Returns user friendly string via getattr if function call fails in main
* Improved error return from JSON request function in amclient.py
* Adds .db and .lck files to .gitignore
* Works at module and executable level
* E402 flake8 warning ignored in Tox to enable module/exe to work
* Captures connection errors in transfers.py
* Initial changes provides scope for future refactoring as required
* Addresses #47
qubot pushed a commit that referenced this issue Mar 2, 2018
@ross-spencer
Refactors amclient.py
e4051bf 
* Creates new logging configuration script to reduce redundancy
* Separates responsibility of amclient, defaults, and cli args
* Cleans up argparse output using metavar
* Adds Conformity to PEP8 to amclient.py and transfers.py
* Returns user friendly string via getattr if function call fails in main
* Improved error return from JSON request function in amclient.py
* Adds .db and .lck files to .gitignore
* Works at module and executable level
* E402 flake8 warning ignored in Tox to enable module/exe to work
* Captures connection errors in transfers.py
* Initial changes provides scope for future refactoring as required
* Addresses #47
qubot pushed a commit that referenced this issue Mar 2, 2018
@ross-spencer
Refactors amclient.py
43c988c 
* Creates new logging configuration script to reduce redundancy
* Separates responsibility of amclient, defaults, and cli args
* Cleans up argparse output using metavar
* Adds Conformity to PEP8 to amclient.py and transfers.py
* Returns user friendly string via getattr if function call fails in main
* Improved error return from JSON request function in amclient.py
* Adds .db and .lck files to .gitignore
* Works at module and executable level
* E402 flake8 warning ignored in Tox to enable module/exe to work
* Captures connection errors in transfers.py
* Adds a utils module to handle json requests across scripts
* Initial changes provides scope for future refactoring as required
* Addresses #47
qubot pushed a commit that referenced this issue Mar 2, 2018
@ross-spencer
Refactors amclient.py
699b018 
* Creates new logging configuration script to reduce redundancy
* Separates responsibility of amclient, defaults, and cli args
* Cleans up argparse output using metavar
* Adds Conformity to PEP8 to amclient.py and transfers.py
* Returns user friendly string via getattr if function call fails in main
* Improved error return from JSON request function in amclient.py
* Adds .db and .lck files to .gitignore
* Works at module and executable level
* E402 flake8 warning ignored in Tox to enable module/exe to work
* Captures connection errors in transfers.py
* Adds a utils module to handle json requests across scripts
* Initial changes provides scope for future refactoring as required
* Addresses #47
@ross-spencer
Copy link
Contributor Author

Closed via 699b018

@ross-spencer ross-spencer mentioned this issue May 17, 2018
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ross-spencer ross-spencer

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@sevein @ross-spencer