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

NIMROD IBEX Script Console: Pylint Errror Loading Script [TIMEBOX: 4 Days] #7034

Closed
2 tasks
JackEAllen opened this issue Feb 23, 2022 · 13 comments
Closed
2 tasks
Assignees
Labels
8 bug re-requested Issue that has been requested again by a user. support

Comments

@JackEAllen
Copy link
Member

JackEAllen commented Feb 23, 2022

Where?

  • Loading scripts into scripting console (genie python).
  • c:\scripts\calib_22_2_22.py

It was found that when loading a script such as c:\scripts\calib_22_2_22.py on NIMROD that pylint failed with w1601

NIMROD_pylintw1601

How?

Occurred when an IS tried to load a script on NIMROD in the scripting console after successfully testing the camera homing routines #6934 . This may be a problem with Pylint as it looks like someone else has experienced the same problem HERE.

If not reproduceable, it may be a problem with IBEX on NIMROD meaning something may have become corrupt.

Reproducible?

[Yes/No]

To Reproduce

  • Load c:\scripts\calib_22_2_22.py found on NIMROD into scripting console in IBEX on a Dev and Test machine.
  • If similar to NIMROD, a pylint error of w1601 will be returned.

Acceptance criteria

  • NIMROD can successfully load scripts including c:\scripts\calib_22_2_22.py in the scripting window of IBEX.
  • A Dev and test machine can also successfully load scripts including c:\scripts\calib_22_2_22.py in the scripting window of IBEX.

How to Test

  • Remote into NIMROD and successfully load the script: c:\scripts\calib_22_2_22.py in the scripting window of IBEX.
  • Repeat the above instruction for both a dev and test machine running IBEX 10.0.0
@JackEAllen
Copy link
Member Author

I can not replicate this on my own personal dev machine. When another test machine becomes available running IBEX 10.0.0 to try and duplicate this issue on, I will update this comment.

@JackEAllen JackEAllen changed the title [NIMROD IBEX Script Console]: [Pylint Errror Loading Script] NIMROD IBEX Script Console: Pylint Errror Loading Script Feb 23, 2022
@rerpha
Copy link
Contributor

rerpha commented Feb 24, 2022

Consider timeboxing this for initial investigation and fix (if there is one and it's not a bug in pylint!)

@rerpha
Copy link
Contributor

rerpha commented Feb 24, 2022

Other things to note:

-pylint from a pip list looked to be 2.12.2 - the latest version

I don't know if we use a pylint.rc file in genie python for the script checking or not.

EDIT: we do - https://github.com/ISISComputingGroup/genie_python/blob/master/Lib/site-packages/genie_python/.pylintrc

@rerpha
Copy link
Contributor

rerpha commented Feb 24, 2022

apply-builtin doesn't appear to be on the list of valid features https://pylint.pycqa.org/en/latest/technical_reference/features.html

but it is in https://pylint.pycqa.org/en/v2.10.2/technical_reference/features.html - so looks to be removed.

@rerpha
Copy link
Contributor

rerpha commented Feb 24, 2022

My theory is we were using this version last time the instrument scientists loaded/checked any scripts and our pylint.rc has got out of date since - we should remove anything that is now not valid.

@rerpha rerpha added the 13 label Feb 24, 2022
@rerpha rerpha changed the title NIMROD IBEX Script Console: Pylint Errror Loading Script NIMROD IBEX Script Console: Pylint Errror Loading Script [TIMEBOX: 4 Days] Feb 24, 2022
@github-actions github-actions bot added ready and removed proposal labels Feb 24, 2022
@KathrynBaker KathrynBaker added this to the SPRINT_2022_02_23 milestone Feb 24, 2022
@rerpha
Copy link
Contributor

rerpha commented Mar 8, 2022

I find it weird that no one else has called us about this... maybe an oddity on NIMROD with an old version of pylint perhaps?

@KathrynBaker
Copy link
Member

Just had the same issue on ZOOM, a script that has been used in the past and was fine but is now giving W1061 error as above

@KathrynBaker KathrynBaker added re-requested Issue that has been requested again by a user. support labels Mar 9, 2022
@rerpha
Copy link
Contributor

rerpha commented Mar 9, 2022

Could you find out what script it was? perhaps there are some similarities between the two offending scripts

@KathrynBaker
Copy link
Member

It was a user script, so I'm a little loathe to put too much info here, but you can find a copy in teams if you search for ZOOM20220309

@JackEAllen
Copy link
Member Author

I can't be certain, but if memory serves me right, I think you can set an environment variable called PYLINTRC pointing the the configuration file you want to read in. might be worth a try.

@KathrynBaker KathrynBaker removed this from the SPRINT_2022_02_24 milestone Mar 24, 2022
@github-actions github-actions bot added the bucket proposals that didn't make into the sprint label Mar 24, 2022
@FreddieAkeroyd
Copy link
Member

I have just pinned pylint to 2.13.9 as 2.14.1 was causing jenkins test failures as per https://stackoverflow.com/questions/72486567/pylint-2-14-0-outputs-bad-option-value-for-no-self-use-no-space-check

@KathrynBaker KathrynBaker removed this from the Sprint_2022_05_19 milestone Jun 16, 2022
@ThomasLohnert ThomasLohnert removed their assignment Jun 17, 2022
@github-actions github-actions bot added the ready label Jun 20, 2022
@KathrynBaker KathrynBaker added this to the Sprint_2022_06_16 milestone Jun 20, 2022
@Tom-Willemsen Tom-Willemsen self-assigned this Jun 21, 2022
@Tom-Willemsen
Copy link
Contributor

Tom-Willemsen commented Jun 23, 2022

I've reproduced this issue locally by copying NIMROD's python installation directly. It appears that when version 10.0.0 was deployed to NIMROD in Jan-2022 the step "backup_directories" was not done correctly (I can see a couple of incomplete backups in c:\data\old\ibex_backup_2022_01_26). This means that the deploy process copied the new python version on top of the old python version, without moving or deleting the old one first.

Between the old and the new version of IBEX, we updated our pylint dependency, which caused one of pylint's internal files to be renamed/refactored. This means that, on NIMROD's python instllation, pylint has both an old (stale) file and a new file defining the same check - which is what the error is complaining about. For anyone curious, the old (stale) file, which should no longer be present in IBEX 10.0.0 but is present on NIMROD, is Instrument\Apps\Python3\Lib\site-packages\pylint\checkers\python3.py.

The description above is also true of ZOOM, the other instrument where we've seen this error.

This also means that other libraries might be in an invalid state on these instruments due to similar stale files.


I propose that the solution to this ticket is:

  • Write a check (nagios?) to assert that Instrument\Apps\Python3\Lib\site-packages\pylint\checkers\python3.py does not exist on the instruments
    • For all instruments which the check flags up, including NIMROD and ZOOM, I think it would be best to redeploy python in it's entirety.
  • Make the deployment process more robust to ensure that stale files from older python versions cannot be left behind even if the move to backup directories fails (might just be as simple as adding a /MIR flag to the robocopy command)

If this is found later on an instrument, it may be sufficient to simply delete Instrument\Apps\Python3\Lib\site-packages\pylint\checkers\python3.py to solve the immediate issue. However we should still consider the python installation "corrupt" and re-install when possible, as it is possible that other modules could also fail with similar causes.

Note: #6369 would be a more general solution and could allow scientists to keep manually-installed packages between ibex releases (which might be why we're not currently using /MIR). This issue has effectively the same cause as #6368

@Tom-Willemsen
Copy link
Contributor

Tom-Willemsen commented Jun 24, 2022

TS2 instruments which have this issue:

  • LET
  • NIMROD
  • OFFSPEC
    • OFFSPEC is on an old version of IBEX, this installation is not corrupt. No action needed.
  • ZOOM

To review the deployments to these instruments, verify that the following files no longer exist on the instruments above:

  • Lib\site-packages\pylint\checkers\python3.py
  • Scripts\iptest3.py

TS1 instruments will receive a new deploy before next running; so I have not patched these.

I've put an example of a corrupt python installation in <share>\ISIS_Experiment_Controls\data for tickets\Ticket7034 if it's useful to be able to compare a before+after.

PRs:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8 bug re-requested Issue that has been requested again by a user. support
Projects
None yet
Development

No branches or pull requests

6 participants