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

break gam.py into smaller files #147

Open
jay0lee opened this issue Dec 23, 2015 · 7 comments
Open

break gam.py into smaller files #147

jay0lee opened this issue Dec 23, 2015 · 7 comments

Comments

@jay0lee
Copy link
Member

jay0lee commented Dec 23, 2015

gam.py is freakin huge at ~360kb. We should break it down into smaller files. First draft thoughts on breakdown:

  • gam.py - init, globals and command parsing
  • admin-sdk/directory/users.py - directory user API calls
  • admin-sdk/directory/groups.py - directory groups API calls
  • drive.py - Drive API calls
  • gmail.py - Gmail API calls
  • gdata-stuff.py - everything related to gdata. Spin this off so we can kill it if Google ever gets an email settings new Google API out the door.

@taers232c thoughts?

@taers232c
Copy link
Contributor

360kb. That's nothing, my full blown version with Contacts, serious error handling, etc, is about 820kb.

It seems to me that one way to break it up is as follows:

Shared stuff: init, globals, utilities, API interface, parsing.
Audit
Calendars/Resource Calendars
ChromeOS/Mobile
Course
Domain/Instance
Drive
Email
Printer/Printjob
Group/OrgUnit/User/Schemas

I'll experiment with my version to get the feel of it.

Ross

ross.scroggs@gmail.com

On Dec 23, 2015, at 5:23 AM, Jay Lee notifications@github.com wrote:

gam.py is freakin huge at ~360kb. We should break it down into smaller files. First draft thoughts on breakdown:

gam.py - init, globals and command parsing
admin-sdk/directory/users.py - directory user API calls
admin-sdk/directory/groups.py - directory groups API calls
drive.py - Drive API calls
gmail.py - Gmail API calls
gdata-stuff.py - everything related to gdata. Spin this off so we can kill it if Google ever gets an email settings new Google API out the door.
@taers232c thoughts?


Reply to this email directly or view it on GitHub.

@jay0lee
Copy link
Member Author

jay0lee commented Aug 21, 2016

@taers232c if we could get gam.py split out it'd make PRs easier and less likely to conflict on merge. For example, if you were doing OAuth changes and they were stored in oauth.py while I'm making changes to Classroom Guardian code stored in classroom_commands.py there would be no conflict (in most cases).

Not to mention, it'd make finding code much easier than searching through gam.py :-)

I took an initial stab at it but it's messier than I'd hoped. Feel free to take a shot if you'd like though.

@crutchcorn
Copy link

It'd also make things like #23 MUCH easier to work with. I'd love to be of help if I can. I might just have to fork this and see what type of damage I can do and report back with a PR and spark some discussion on the matter. I'd love to know any formatting or ideas you guys have (you know, being the owners and contributors and all), so that I can help contribute to the project upstream.

@taers232c
Copy link
Contributor

Corbin,

This will not be easy, it is way beyond a PR, it's a rewrite. I'll look at
it again, maybe a bolt of lightning/inspiration will hit me.

I have changed by advanced version to be pseudo-importable:
https://github.com/taers232c/GAMADV-X/releases

#!/usr/bin/env python

import os, sys, shlex

from gam import ProcessGAMCommand

Set appropriate values

os.environ['GAMCFGDIR'] = '/Users/admin/.gam'

GAM_STDOUT = '/tmp/gamstdout.out'

Choose how to produce args list

rc = ProcessGAMCommand(shlex.split('/Users/admin/GAM/gam.py redirect stdout
{0} info user admin'.format(GAM_STDOUT)))

#rc = ProcessGAMCommand(['/Users/admin/GAM/gam.py',

'redirect', 'stdout', GAM_STDOUT,

'info', 'user', 'admin'])

print 'GAM returned {0}'.format(rc)

with open(GAM_STDOUT, 'rU') as f:

for line in f:

    sys.stdout.write(line)

Ross

On Wed, Nov 2, 2016 at 2:09 PM, Corbin Crutchley notifications@github.com
wrote:

It'd also make things like #23 #23
MUCH easier to work with. I'd love to be of help if I can. I might just
have to fork this and see what type of damage I can do and report back with
a PR and spark some discussion on the matter. I'd love to know any
formatting or ideas you guys have (you know, being the owners and
contributors and all), so that I can help contribute to the project
upstream.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#147 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AIU8L9Vc1UCckj_bpsbcW4QtJjhtBvawks5q6PwSgaJpZM4G6jnq
.

Ross Scroggs
ross.scroggs@gmail.com

@ollytheninja
Copy link

Hi! I've been using this tool for a couple of bits of automation and then almost died when I saw the mono-file architecture. (don't get me wrong, I completely understand how it got to this point!)

I'd like to help, the main reason is to create a nicer, more pythonic API for people (like myself) wanting to use GAM as a library for their own python scripts.

My suggestion would be that gam.py retains all it's current function signatures but as the logic is pulled out into other files, these would just call into those new places. This would retain existing functionality in scripts written against the current version. Once that is complete, the gam.py file could be deprecated and documentation updated to point at the new classes / methods.

My questions are:

  • is anyone else working on this at the moment?
  • are there strong opinions about what a new, from scratch API would look like? other than naming being pep8 compliant?

@ejochman
Copy link
Contributor

ejochman commented Dec 1, 2019

I can take a stab at starting to break things apart. I generally agree with the logical split that @jay0lee laid out in the OP. I'll start in that direction and you all can feel free to course correct me, as needed, as the PRs come in.

I'm not going to commit to a timeline to doing this, but I'll try to start working on it with what free time I have. This is going to take a while, and I will be purposefully taking out bite-size chunks to make the migration easier to understand and avoid breakage. At the same time, I'll try to add docstrings and push towards making the style more PEP8 compliant, along with adding unit tests where possible. I'm going to avoid rewriting the existing logic as much as possible on this read through, but it will probably necessary in order to untangle some things.

(PS: To anyone who sees this comment in 3+ years when this isn't finished, I'm sorry!)

ejochman added a commit to ejochman/GAM that referenced this issue Dec 3, 2019
Start with one of the deepest parts of the stack, Google API request execution calls and associated errors. Critical information printing functions and application control logic are also broken out into their own components.

This change also adds unit tests for migrated content and makes code more PEP8 compliant.

This commit starts work on  GAM-team#147
jay0lee pushed a commit that referenced this issue Dec 7, 2019
* Begin breaking apart gam.py into logical pieces

Start with one of the deepest parts of the stack, Google API request execution calls and associated errors. Critical information printing functions and application control logic are also broken out into their own components.

This change also adds unit tests for migrated content and makes code more PEP8 compliant.

This commit starts work on  #147

* Add unit tests to Travis config

* Swap assert_called_once() with assertEqual() and Mock.call_count

Makes tests compatible with Python 3.5. assert_called_once() is only available in Python 3.6+
ejochman added a commit to ejochman/GAM that referenced this issue Dec 9, 2019
ejochman added a commit to ejochman/GAM that referenced this issue Dec 9, 2019
ejochman added a commit to ejochman/GAM that referenced this issue Dec 9, 2019
ejochman added a commit to ejochman/GAM that referenced this issue Dec 10, 2019
jay0lee pushed a commit that referenced this issue Dec 20, 2019
* Move callGAPIItems and add unit tests

* Move the page-related call() methods and add tests

Continues work on #147

* Account for rand addition that rounds to 1.0

In this case, sleep time could be equal to 61
@jay0lee
Copy link
Member Author

jay0lee commented Mar 8, 2020

I just pushed:

e1d76a9

which is an attempt to start breaking the actual GAM command functions into their own file. I expect that this will roughly equate to one file per-API though there are some APIs like Directory where we might want to do one API file per object of the API (users, groups, mobiledevices, etc in the Directory API case).

I cheated somewhat to get this done, import __main__ is rather hacky and may give us problems down the road, it's definitely not something I hope to see stick around long. I'd also like to find a better solution than from var import * in each file to share global vars.

But for now, the main task is to go ahead and pull the rest of the GAM command functions (functions that for all intent purposes correspond 1:1 with a GAM command) into their own API files. Once that's done we can look at what's left of gam.py and figure out where it should go while also figuring out what a bare minimum gam.py will look like.

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

No branches or pull requests

5 participants