Skip to content
This repository has been archived by the owner on Aug 8, 2018. It is now read-only.

Use external model references instead of inlining #81

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arabold
Copy link

@arabold arabold commented Nov 11, 2015

This pull request addresses issue #43. Instead of inlining all model definitions, we use external references now. This way we are reusing models which reduces the amount of classes and code generated in SDKs.

My main concern about this pull request: This code effectively removes inlining. Is there any case in which inlining would be preferred over external references?

@HyperBrain
Copy link

Is there any news on this? For us it works, but is there any shortcoming as @arabold mentioned?

@KraigM
Copy link

KraigM commented Dec 19, 2015

This solution isn't working for me. The code works perfectly but API Gateway does not like "https://apigateway.amazonaws.com/restapis/APP_ID/models/User" even though there is a 'User' model on "APP_ID". Is this still working for you guys?

@arabold
Copy link
Author

arabold commented Dec 20, 2015

It was still working perfectly fine last week. We're making heavy use of models and external references. When is it giving an error and what's the specific message?

@arabold
Copy link
Author

arabold commented Jan 18, 2016

Is there anything I can help to have this merged into the main repository?

@rpgreen
Copy link
Contributor

rpgreen commented Apr 4, 2016

Since API Gateway validates the existence of references in your model schemas, the models must be imported in the correct order from the "leaf" nodes upwards. The import will fail if the importer attempts to create a model before its dependent reference model is imported.

This is most likely why this is failing for @KraigM.

I will merge this if you can update so that the models are guaranteed to be imported in the correct order, and resolve conflicts.

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

Successfully merging this pull request may close these issues.

None yet

4 participants