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

feat(AclFamily): add acl whoami command #1774

Merged
merged 3 commits into from
Sep 1, 2023
Merged

Conversation

kostasrim
Copy link
Contributor

  • add acl whoami
  • add tests

@kostasrim kostasrim changed the base branch from main to acl_part_7_add_acl_deluser August 30, 2023 16:46
@kostasrim kostasrim self-assigned this Aug 30, 2023
dranikpg
dranikpg previously approved these changes Aug 30, 2023
Comment on lines +227 to +283
@pytest.mark.asyncio
async def test_acl_whoami(async_client):
await async_client.execute_command("ACL SETUSER kostas >kk +@ALL ON")

with pytest.raises(redis.exceptions.ResponseError):
await async_client.execute_command("ACL WHOAMI WHO")

result = await async_client.execute_command("ACL WHOAMI")
assert result == "User is default"

result = await async_client.execute_command("AUTH kostas kk")

result = await async_client.execute_command("ACL WHOAMI")
assert result == "User is kostas"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please stop using pytests 😆

Copy link
Collaborator

@romange romange Aug 31, 2023

Choose a reason for hiding this comment

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

I agree. This type of logic we test with unit tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I promise I will move these to units next week after the release ❤️

Base automatically changed from acl_part_7_add_acl_deluser to main September 1, 2023 16:12
@kostasrim kostasrim dismissed dranikpg’s stale review September 1, 2023 16:12

The base branch was changed.

@@ -220,7 +218,7 @@ async def test_acl_deluser(df_server):


script = """
for i = 1, 10000 do
for i = 1, 100000 do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I slightly increased this to avoid flakes but what I run locally won't necessary reflect the gh runners since the machines we are running on the regressions tests are far more powerful than mine

Copy link
Contributor

Choose a reason for hiding this comment

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

They're less powerful. The flakes I was worried about is that it setuser migth run to quickly

@kostasrim kostasrim enabled auto-merge (squash) September 1, 2023 17:37
@kostasrim kostasrim merged commit 68fa3f4 into main Sep 1, 2023
10 of 14 checks passed
@kostasrim kostasrim deleted the acl_part_8_add_whoami branch September 1, 2023 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants