search

pitt-crc / wrappers

User focused command line wrappers around Slurm
https://crc-pages.pitt.edu/wrappers/
GNU General Public License v3.0
1 stars 1 forks source link

Update crc_squeue #231

Closed chnixi closed 3 months ago

chnixi commented 5 months ago

Added warning message to notify users within 30 days of proposal expiration date.

Comeani commented 5 months ago

@chnixi There were some issues with the imports and some syntax errors that I fixed.

The utility functions for touching keystone have been changed a bit since this last had main merged in. They should still return the right things, I think I just changed the names to be more accurate. Can you make the corresponding changes here?

chnixi commented 5 months ago

Not sure why it is giving the error "AttributeError: 'function' object has no attribute 'getuser". Did we change something in getpass? crc-scancel uses the same routine to get the user name.

Comeani commented 5 months ago

from getpass import getpass

This is not importing getpass the module, but getpass the function inside the getpass module.

You can change the import to from getpass import getpass, getuser and the change line 47 to user = f'-u {getuser()}'

chnixi commented 5 months ago

Fixed this, Now it is complaining about: 'username': os.environ["USER"], File "/usr/lib64/python3.8/os.py", line 675, in getitem raise KeyError(key) from None KeyError: 'USER'

The call where this happens is the same as in crc-usage:
auth_header = get_auth_header(KEYSTONE_URL, {'username': os.environ["USER"], 'password': getpass("Please enter your CRC login password:\n")})

Comeani commented 5 months ago

Is the $USER environment variable defined in the shell you are running from? I'll see if I can reproduce the problem

Comeani commented 5 months ago

os.environ does not give me any issues running on the cluster, I fixed the imports so they work properly now.

I see where you ran into that problem. It looks like the tests break in the CI when trying to find the USER environment variable. I'll see if I can find a fix for this.

Comeani commented 5 months ago

Adding the call to keystone makes it impossible to run some of these tests properly because the CI can't get the password for authenticating, and even if it could it wouldn't be able to reach out deployment of keystone.

I think we make an issue to rework utils so it can do it's functionality independent from the application logic for any given wrapper (Daniel gave me some note on how I might start doing this) and be more easily mocked in these tests.

If we can test that the output for a expiring allocation shows up for this version of crc-squeue on the cluster, I'm content to merge it in and revisit the tests another time (or just defer until we have email notifications in place)