-
Notifications
You must be signed in to change notification settings - Fork 0
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
Update location of sybpydb for prime Python 3.10 #22
Conversation
@@ -91,7 +91,7 @@ def __init__(self, dbi=None, server=None, user=None, passwd=None, database=None, | |||
raise ValueError( | |||
'SYBASE_OCS env variable not defined. sybase dbi works only in production ska.') | |||
modulepath = os.path.join(os.environ['SYBASE'], os.environ['SYBASE_OCS'], | |||
'python', 'python34_64r', 'lib') | |||
'python', 'python39_64r', 'lib') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this, I could do something to check the python version and then select the right directory, but I figured we didn't need updates to this to be back-compatible with older Python versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we would want it to be compatible with the current version, right? Will this PR work with the current flight version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we necessarily need it to be compatible with the current python version, as we don't do "mixed' environments (ska3-flight for Python 3.10.8 won't have an option to work with a different/older ska3-core) and we can control this by just promoting this change with ska3-prime. If we are actually worried about it, I can add the logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @jeanconn is proposing that this PR be included only in the ska3-prime release. We generally don't require those packages to be back-compatible with earlier Python versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, but as soon as this is merged masters tests will be broken until we have a ska3-prime release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Along those lines though, does it make sense to update masters testing to work from a ska3 prime candidate after 2022.13 is promoted anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose we should.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Description
Update location of sybpydb to support new release (requires Python >= 3.9).
Fixes #
Interface impacts
Testing
Unit tests run in an early build of ska3-prime / ska3-core with confirmed Python 3.10.8 .
Unit tests
Functional tests
No functional testing.