openfisca / openfisca-web-api

[DEPRECATED] Web API for OpenFisca
https://www.openfisca.fr/
GNU Affero General Public License v3.0
13 stars 11 forks source link

Internationalize the API #78

Closed fpagnoux closed 7 years ago

fpagnoux commented 7 years ago

Connected to #76.

Depends on https://github.com/openfisca/openfisca-core/pull/436 Depends on #81

Some of the API-related business is done in the scenarios. I'll generalise it later in a more complex PR, but this is enough to make the api work with senegal (provided that the scenario has been copy pasted and adapted from france).


Todo :

fpagnoux commented 7 years ago

To reproduce: core#4.3.2 senegal#0.3.1 web-api#580af38c1a3442b395dd63124bce4bafa2c71987

Just try to serve the api with paster serve api/api_config.ini from the senegal repo.

fpagnoux commented 7 years ago

Stack trace:

  File "/Users/fpagnoux/dev/mes-aides/openfisca/openfisca/web-api/openfisca_web_api/application.py", line 71, in make_app
    environment.load_environment(global_conf, app_conf)
  File "/Users/fpagnoux/dev/mes-aides/openfisca/openfisca/web-api/openfisca_web_api/environment.py", line 148, in load_environment
    model.get_cached_or_new_decomposition_json(tax_benefit_system)
  File "/Users/fpagnoux/dev/mes-aides/openfisca/openfisca/web-api/openfisca_web_api/model.py", line 42, in get_cached_or_new_decomposition_json
    xml_file_path = os.path.join(tax_benefit_system.DECOMP_DIR, xml_file_name)
AttributeError: 'SenegalTaxBenefitSystem' object has no attribute 'DECOMP_DIR'
fpagnoux commented 7 years ago

Since https://github.com/openfisca/openfisca-core/pull/397, we put DEFAULT_DECOMP_FILE as a general attribute of a TaxBenefitSystem. This is what mislead this condition.

fpagnoux commented 7 years ago

See #81

cbenz commented 7 years ago

@fpagnoux I cannot ask you to review since you're the author of the PR.

What should I do? Recreate a new PR replacing this one, or asking you to review in normal comments?

This pattern happens often.

fpagnoux commented 7 years ago

Asking a review in regular comments seems good to me in this case.

In which context did you encounter the error ? With which route ? I don't remember running into something about a description.

fpagnoux commented 7 years ago

Tested and approved with senegal. Seems GTM for me.

cbenz commented 7 years ago

About descriptions as said in the Changelog, this is about the parameters endpoint, which returned empty strings when no description was set; there was no exception.

This did not happen with France and I saw it on Senegal.

@fpagnoux Can you merge? You're the author of the PR ;-)


In terms of practices used in the API:

good:

{}

bad:

{
  "description": null
}

ugly:

{
  "description": ""
}