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

Resolve Case Sensitivity Issue in ZenML Stack Renaming #2258

Open
1 task
strickvl opened this issue Jan 10, 2024 · 13 comments
Open
1 task

Resolve Case Sensitivity Issue in ZenML Stack Renaming #2258

strickvl opened this issue Jan 10, 2024 · 13 comments
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@strickvl
Copy link
Contributor

Open Source Contributors Welcomed!

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

Contact Details [Optional]

[email protected]

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

  • The backend should allow users to rename stacks with changes in letter case (e.g., from “Stack” to “stack”).
  • The validation check for stack renaming should be case-sensitive to differentiate between such names accurately.
  • Resolving this issue will enhance stack management flexibility and adhere to user preferences for naming conventions.

Steps to Implement

  • Analyze the stack renaming validation logic in the ZenML backend. (Start with the CLI but then also the ZenStore + Client methods involved in this).
  • Modify the validation process to be case-sensitive, correctly distinguishing between names like “Stack” and “stack.”
  • Conduct thorough testing to ensure that the updated logic allows for case-sensitive renaming and does not introduce other issues. (Add tests to the PR).
  • Update documentation, if necessary, to indicate the improved handling of stack renaming with respect to case sensitivity.

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

  • I agree to follow this project's Code of Conduct
@strickvl strickvl added bug Something isn't working good first issue Good for newcomers labels Jan 10, 2024
@amitsgh
Copy link

amitsgh commented Jan 10, 2024

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

@strickvl
Copy link
Contributor Author

@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
Copy link

sandeepreddygantla commented Jan 12, 2024

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
Copy link
Contributor Author

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
Copy link

amitsgh commented Jan 13, 2024

@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
Copy link
Contributor Author

@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
Copy link

amitsgh commented Feb 16, 2024

@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.

@amitsgh amitsgh removed their assignment Feb 16, 2024
@Ashwin143
Copy link

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

@Ashwin143
Copy link

Screenshot 2024-04-20 at 2 17 53 PM

Unable to load this page

@htahir1
Copy link
Contributor

htahir1 commented Apr 20, 2024

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

@Ashwin143
Copy link

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
Copy link

"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
Copy link

Prabhat-Thapa45 commented Oct 2, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

6 participants