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

adding new "describe_api" command #1092

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

Conversation

sesas
Copy link

@sesas sesas commented Dec 8, 2013

Request for comments and feedback.

@sesas
Copy link
Author

sesas commented Dec 8, 2013

How do I tie an issue to this? https://github.com/toastdriven/django-tastypie/issues/1091

@sesas
Copy link
Author

sesas commented Dec 8, 2013

Could I add myself to the AUTHORS is this is pulled?

@georgedorn
Copy link
Contributor

You should add yourself to AUTHORS as part of this pull request.

@sesas
Copy link
Author

sesas commented Dec 8, 2013

@georgedorn thank you for your answer :-)

RESOURCE_POSTFIX = 'Resource'


def describe_api(app, **options):
Copy link
Contributor

Choose a reason for hiding this comment

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

This function could use tests. Those would also help explain what this does and what to expect from it.

There are several test projects in the top-level tests module, each with apps and models already defined, so you could just add tests to an existing test project.

Copy link
Author

Choose a reason for hiding this comment

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

So are you suggesting that I should pick one of those sample apps (say basic) run the main function on it's app, write the output of that function to a file called api.py and then run some GET, POST, PUT, and DELETE http requests to it?

Maybe I should copy the test/basic to test/describe_api first, delete the current api/resources.py and modify the api/urls.py then.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm suggesting a test that uses the call_command() function to run the
command using one of the test projects' apps as input.

Sorta like how django has tests for the inspectdb command (
https://github.com/django/django/blob/master/tests/inspectdb/tests.py).

On Sat, Dec 7, 2013 at 6:00 PM, sesas notifications@github.com wrote:

In tastypie/management/commands/describe_api.py:

+class Command(AppCommand):

  • help = "Generates tastypie resources for the given app that you should put in APP/api.py."
  • args = "[appname]"
  • label = "application name"
  • def handle_app(self, app, **options):
  •    # raise NotImplementedError()
    
  •    return describe_api(app)
    
    +FIELD_TEMPLATE = "{attribute_name} = fields.ForeignKey('{app_name}.api.{foreign_model}', '{attribute_name}'{options_str})"
    +RESOURCE_POSTFIX = 'Resource'
    +
    +
    +def describe_api(app, **options):

So are you suggesting that I should pick one of those sample apps (say
basic) run the main function on it's app, write the output of that
function to a file called api.py and then run some GET, POST, PUT, and
DELETE http requests to it?

Maybe I should copy the test/basic to test/describe_api first, delete the
current api/resources.py and modify the api/urls.py then.


Reply to this email directly or view it on GitHubhttps://github.com/toastdriven/django-tastypie/pull/1092/files#r8183568
.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the info and clarification. Do you think it would be necessary to test the auto-generated apis as well, or just making sure the string that comes out of it is the same as a predetermined one?

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

Successfully merging this pull request may close these issues.

None yet

3 participants