omkar-foss commented on code in PR #43083:
URL: https://github.com/apache/airflow/pull/43083#discussion_r1803379283


##########
tests/api_fastapi/core_api/routes/public/test_variables.py:
##########
@@ -136,6 +136,34 @@ def test_get_should_respond_404(self, test_client):
         assert f"The Variable with key: `{TEST_VARIABLE_KEY}` was not found" 
== body["detail"]
 
 
+class TestGetVariables(TestVariableEndpoint):
+    @pytest.mark.enable_redact
+    @pytest.mark.parametrize(
+        "query_params, expected_total_entries, expected_keys",
+        [
+            # Filters
+            ({}, 3, [TEST_VARIABLE_KEY, TEST_VARIABLE_KEY2, 
TEST_VARIABLE_KEY3]),
+            ({"limit": 1}, 3, [TEST_VARIABLE_KEY]),
+            ({"limit": 1, "offset": 1}, 3, [TEST_VARIABLE_KEY2]),
+            # Sort
+            ({"order_by": "id"}, 3, [TEST_VARIABLE_KEY, TEST_VARIABLE_KEY2, 
TEST_VARIABLE_KEY3]),
+            ({"order_by": "-id"}, 3, [TEST_VARIABLE_KEY3, TEST_VARIABLE_KEY2, 
TEST_VARIABLE_KEY]),
+            ({"order_by": "key"}, 3, [TEST_VARIABLE_KEY3, TEST_VARIABLE_KEY2, 
TEST_VARIABLE_KEY]),
+            ({"order_by": "-key"}, 3, [TEST_VARIABLE_KEY, TEST_VARIABLE_KEY2, 
TEST_VARIABLE_KEY3]),

Review Comment:
   >This does not appear to be the case when I run the server locally and test 
manually, just in tests, which I find really weird.
   
   This could probably be because on local config it'd using the fernet key 
with `is_encrypted = False`, while in test config as you mentioned it's using a 
dynamically generated fernet key.
   
   >If someone has an idea.
   
   Since encrypted values are getting sorted, doesn't look like sort by `value` 
would be useful to the user. Also if values are large then sorting them could 
degrade performance.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to