[GitHub] [airflow] houqp commented on pull request #9273: implement api v1 for variables

2020-06-17 Thread GitBox
houqp commented on pull request #9273: URL: https://github.com/apache/airflow/pull/9273#issuecomment-645722927 @mik-laj updated and ready for another round of review This is an automated message from the Apache Git Service.

[GitHub] [airflow] houqp commented on pull request #9273: implement api v1 for variables

2020-06-14 Thread GitBox
houqp commented on pull request #9273: URL: https://github.com/apache/airflow/pull/9273#issuecomment-643811896 > I think, writing tests improve code coverage? As @OmairK already mentioned, duplicated tests that test the same code path won't increase code coverage. In fact it will

[GitHub] [airflow] houqp commented on pull request #9273: implement api v1 for variables

2020-06-13 Thread GitBox
houqp commented on pull request #9273: URL: https://github.com/apache/airflow/pull/9273#issuecomment-643675092 @OmairK the api tests should have already covered serialization and deserialization code path, no? Do you have an example in mind where a refactor will cause issue that won't be

[GitHub] [airflow] houqp commented on pull request #9273: implement api v1 for variables

2020-06-13 Thread GitBox
houqp commented on pull request #9273: URL: https://github.com/apache/airflow/pull/9273#issuecomment-643594204 > When a service receives an immutable field in an update request (or similar), even if included in the update mask, the service should ignore the field if the value matches, but

[GitHub] [airflow] houqp commented on pull request #9273: implement api v1 for variables

2020-06-13 Thread GitBox
houqp commented on pull request #9273: URL: https://github.com/apache/airflow/pull/9273#issuecomment-643582917 @mik-laj the in openapi spec, the PATH method for variable takes the following body: ``` PATCH /api/v1/variables/foo {"key": "foo", "value": "bar"} ``` I