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

Skip report vv warn if no sybase passwd file for db #256

Merged
merged 2 commits into from
Jan 13, 2021
Merged

Conversation

jeanconn
Copy link
Contributor

Description

Skip report vv warn if no sybase passwd file for db

Testing

  • Passes unit tests on MacOS, linux, Windows (at least one required)
  • Functional testing
    • ran as jeanconn and kadi user on Linux from production ska3 and see test pass and test skip as appropriate

Fixes #

@jeanconn
Copy link
Contributor Author

More klunky compromise here, but I think this keeps me happy about test coverage and could stop the warns (though now @javierggt would need to delete his passwd file if it isn't actually working for test to pass... and put it back when it works).

Copy link
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

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

Both of the changes are not required, but are about learning (through usage) new and better patterns for doing common things.

@@ -9,16 +9,21 @@

from .. import report

user = os.environ.get('USER') or os.environ.get('LOGNAME')
Copy link
Member

Choose a reason for hiding this comment

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

I think the best way to get user information is this, which you can reliably use on any system:

import getpass
getpass.getuser()

HAS_SYBASE_ACCESS = False

# If the user should have access, warn about the issue.
if (on_head_network() and not has_sybase() and
os.path.exists(os.path.join(os.environ['SKA'], 'data', 'aspect_authorization',
Copy link
Member

Choose a reason for hiding this comment

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

You should just start using Path at every opportunity. It is very much nicer in almost every case:
Path(os.environ['SKA'], 'data', 'aspect_authorization', f'sqlsao-axafvv-{user}').exists().

@jeanconn
Copy link
Contributor Author

Sure. Yes, I was just extending what was there (at least in my mind) instead of doing it a little better. I can do it a little better.

@jeanconn jeanconn merged commit cbc3622 into master Jan 13, 2021
@jeanconn jeanconn deleted the forgive-test branch January 13, 2021 21:29
@jeanconn
Copy link
Contributor Author

@javierggt up to you if you want to leave your sqlsao-axafvv-* file around or if you want to delete so the tests will pass against master until vv access actually works.

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.

2 participants