Skip to content

Lock files cannot be read in the live queue #13

Closed
campb303 opened this issue Mar 17, 2021 · 6 comments
Closed

Lock files cannot be read in the live queue #13

campb303 opened this issue Mar 17, 2021 · 6 comments
Assignees
Labels
bug An issue that results in API breaking question Something that requires more information before moving forward quickfix Immediately actionable and should be fast

Comments

@campb303
Copy link
Collaborator

When editing files using q-cli, lock files are created to prevent items from being edited by the another person. These files exist in the same directory as the original item with the number of the item followed by .lck. For example, if I were to edit ce 40 the directory structure would look like this:

ce/
├── 40
└── 40.lck

The following permissions are expected:

-rw-r--r--  1 campb303 ecnuser    0 Jan 19 16:32 40
-rw-r--r--  1 campb303 ecnuser    0 Jan 19 16:32 40.lck

When gizmo created lock files, his permissions are set to 600 instead of 644 but I don't know why.

# Same for qedit and qtedit
-rw-------  1 gizmo ecnqueue      51 Jan 19 16:12 69.lck

When I, @benne238, other students and/or full timers create lock files permission get set to 644. I believe this issue is just with gizmo.

@campb303 campb303 added bug An issue that results in API breaking question Something that requires more information before moving forward labels Mar 17, 2021
@campb303 campb303 added this to the v2-production-ready milestone Mar 17, 2021
@campb303
Copy link
Collaborator Author

gizmo told me he has his user's mask set to 0077 in his .bashrc file which explains the above behavior.

A potential solution here could be to change the live queue directory's group to ecnqueue and set a sticky bit to make all files in that directory group readable. Need to check with Sundeep and Curtis.

@campb303
Copy link
Collaborator Author

Next steps are to test group sticky perms to see if this fixes the issue.

@campb303 campb303 modified the milestones: production-ready, write-access May 17, 2021
@campb303
Copy link
Collaborator Author

campb303 commented Jun 2, 2021

gizmo should change his umask setting in his .bashrc file on pier to avoid this issue.

@campb303 campb303 removed this from the write-access milestone Jul 6, 2021
@campb303 campb303 self-assigned this Jul 6, 2021
@campb303
Copy link
Collaborator Author

campb303 commented Jul 6, 2021

Currently the locked status of an item is established by reading the contents of a lock file. This creates an issue when a lock file exists but cannot be read like in the case of gizmo's non-default mask.

The behavior should be changed from reading the contents of the lock file to checking for the pressence of a lock file. If a lock file exists then the item is assumed to be locked by the owner of the lock file.

This means we will lose the ability to know what tool is being used to lock the file. For the time being this won't be an issue but might need revisted if more tools are used to interact with the queue.

@benne238 benne238 added the quickfix Immediately actionable and should be fast label Jul 12, 2021
@benne238 benne238 self-assigned this Jul 12, 2021
@benne238
Copy link
Collaborator

Fix

To fix this issue, the pathlib.Path class can take a file path, and find the owner of the file using a simple function. The following changes were made to __check_is_locked() in the Item class:

def __check_is_locked(self) -> Union[str, bool]:
        """Returns a string info about the lock if true and a bool False if false

        Example: A file is locked
                "CE 100 is locked by campb303 using qvi"

        Example: a file is not locked
                False

        Returns:
                Union[str, bool]: String with info about lock if true, bool False if false
        """
        lock_file = str(self.path) + ".lck"
        if os.path.exists(lock_file):
-           with open(lock_file) as file:
-               lock_info = file.readline().split(" ")
-               locked_by = lock_info[4]
-               locked_using = lock_info[1]
-               return f"{self.queue} {self.number} is locked by {locked_by} using {locked_using}"
+           lock_file = Path(lock_file)
+           lock_file_owner = lock_file.owner()
+           return f"{self.queue} {self.number} is locked by {lock_file_owner}."
        else:
            return False

This change avoids attempting to read a file that might not have the correct permissions

@campb303
Copy link
Collaborator Author

campb303 commented Aug 2, 2021

Closed by #40

@campb303 campb303 closed this as completed Aug 2, 2021
Sign in to join this conversation on GitHub.
Labels
bug An issue that results in API breaking question Something that requires more information before moving forward quickfix Immediately actionable and should be fast
Projects
None yet
Development

No branches or pull requests

2 participants