zenml-io / zenml

ZenML 🙏: The bridge between ML and Ops. https://zenml.io.
https://zenml.io
Apache License 2.0
4.04k stars 436 forks source link

Resolve Case Sensitivity Issue in ZenML Stack Renaming #2258

Open strickvl opened 9 months ago

strickvl commented 9 months ago

Open Source Contributors Welcomed!

Please comment below if you would like to work on this issue!

Contact Details [Optional]

support@zenml.io

What happened?

There is a case sensitivity issue in the backend of ZenML when users attempt to rename a stack from uppercase to lowercase (or vice versa) (using the frontend dashboard). For example, changing a stack name from “Stack” to “stack” is not possible. The backend's validation logic for renaming stacks is case-insensitive, leading it to mistakenly identify the new lowercase name as a duplicate of the existing uppercase name.

Task Description

Revise the backend validation logic in ZenML to properly handle case sensitivity in stack renaming. The system should permit changes in the case of stack names without erroneously recognizing them as duplicates.

Expected Outcome

Steps to Implement

Additional Context

This enhancement is critical for users who rely on specific naming conventions for their stacks, where case sensitivity can be an essential aspect of their organizational or compliance requirements.

Code of Conduct

amitsgh commented 9 months ago

Hello @strickvl, I'm interested in working on this issue.

strickvl commented 9 months ago

@amitsgh You'd be most welcome to do so! Let us know if you have any questions about the scope etc, and please make sure to follow our contributor's guide (esp the part about basing your branch and PR off the develop branch of your fork). Thanks!

sandeepreddygantla commented 9 months ago

Let's say the current validation logic looks something like this (simplified for illustration):

def is_duplicate_name(existing_names, new_name):
    """Check if the new stack name already exists (case-insensitive)."""
    return new_name.lower() in (name.lower() for name in existing_names)

To make it case-sensitive, you would update the function like this:

def is_duplicate_name(existing_names, new_name):
    """Check if the new stack name already exists (case-sensitive)."""
    return new_name in existing_names

Additionally, you'll need to ensure that the rest of the ZenML stack management system aligns with this case-sensitive approach. This includes areas where stacks are listed, retrieved, and stored.

For the testing part, you would write unit tests to verify the behavior:

import unittest

class TestStackRenaming(unittest.TestCase):

    def test_case_sensitive_renaming(self):
        existing_names = ['Stack', 'TestStack', 'Example']
        self.assertFalse(is_duplicate_name(existing_names, 'stack'))
        self.assertTrue(is_duplicate_name(existing_names, 'Stack'))
        self.assertFalse(is_duplicate_name(existing_names, 'example'))
        self.assertTrue(is_duplicate_name(existing_names, 'Example'))

if __name__ == '__main__':
    unittest.main()

This test suite checks that the renaming logic is now sensitive to case changes. It's a basic example and should be expanded based on the full range of functionality and edge cases in the ZenML stack management system.

strickvl commented 9 months ago

Hi! The code snippets you mention don't really have anything to do with the ZenML code base. I'd encourage you to check out the Client methods involved in stack renaming and the others mentioned in the beginning of the 'Steps to Implement' section. You might also want to take a look at existing tests inside the tests/ folder which cover how stack renaming currently happens. In particular, this test is probably what you want to work against, but it could be reworked for this specific case as follows:

def test_renaming_stack_with_update_method_succeeds(clean_client):
    """Tests that renaming a stack with the update method succeeds."""
    stack = _create_local_stack(
        client=clean_client, stack_name="My_stack_name"
    )
    clean_client.activate_stack(stack.id)

    new_stack_name = "new_stack_name"

    # use the specific exception that gets raised instead of the generic exception
    with pytest.raises(Exception):
        clean_client.update_stack(
            name_id_or_prefix=stack.id, name=new_stack_name
        )
    assert not clean_client.get_stack(name_id_or_prefix=new_stack_name)

Is that any clearer?

amitsgh commented 9 months ago

@strickvl, I don't seem to be assigned to this task. Could you please check and assign it to me so that it appears in my assigned list?

strickvl commented 9 months ago

@amitsgh sure. We use the assignments a bit differently at ZenML but I assigned you anyway since it seems it's important to you!

amitsgh commented 8 months ago

@strickvl, I apologize for the delay in addressing the GitHub issue. Unfortunately, I'm currently unable to dedicate time to it. I'm sorry for any inconvenience this may cause. Additionally, I will unassign myself from the issue to allow someone else to take over.

Ashwin143 commented 6 months ago

Hello @strickvl, I'm interested in working on this issue, if it is not resolved may i check

Ashwin143 commented 6 months ago
Screenshot 2024-04-20 at 2 17 53 PM

Unable to load this page

htahir1 commented 6 months ago

@Ashwin143 https://docs.zenml.io/user-guide/production-guide/understand-stacks

Ashwin143 commented 6 months ago

though this is not related to this issue i just want to bring it to your notice, when i click on the docs it is leading to error link instead of leading to correct hyperlink you gave above

Screenshot 2024-04-20 at 6 39 40 PM
Ashwin143 commented 3 months ago

"Hi team, I apologize for the delay in addressing this PR. I was briefly away working on another project and could use some help getting reoriented with the project structure. Would someone be able to provide a quick overview or point me to relevant resources so I can finalize this contribution?"

Prabhat-Thapa45 commented 1 month ago

is this issue still valid @strickvl, since I see no option to update the name as of now

Image 02-10-2024 at 13 46