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

Failure to correct poetry paths after update #2757

Closed
FroggyFlox opened this issue Dec 1, 2023 · 2 comments · Fixed by #2771
Closed

Failure to correct poetry paths after update #2757

FroggyFlox opened this issue Dec 1, 2023 · 2 comments · Fixed by #2771
Assignees

Comments

@FroggyFlox
Copy link
Member

On my personal Rockstor machine, I recently updated from a pre-Poetry version (4.1.0) to latest Testing (5.0.5). Despite the shameful failure to keep that machine up-to-date until now, the update went without a hiccup.
Since then, however, I did receive an email notification that my scheduled task failed:

/bin/bash: /opt/rockstor//bin/st-pool-scrub: No such file or directory

This look exactly like what was supposed to be corrected by #2558: migrating previous path to poetry venv. After a closer look, we can see that my previous (currently failing) path contains a double // (not sure why). This is why our mechanism to update to poetry paths implemented in #2558 failed. We are indeed looking for the presence of the full path with only single / and thus fail to match that pattern:

>>> import re
>>> pattern = "/opt/rockstor/bin/"
>>> line="42 3 * * 5 root /opt/rockstor//bin/st-pool-scrub 1 \*-*-*-*-*-*"
>>> re.search(pattern, line)
>>>

We may want to update that pattern to:

>>> pattern = "/opt/rockstor[/]+bin/"
>>> re.search(pattern, line)
<re.Match object; span=(16, 35), match='/opt/rockstor//bin/'>
@phillxnet phillxnet added this to the 5.1.X-X Stable release milestone Dec 2, 2023
@FroggyFlox FroggyFlox self-assigned this Dec 18, 2023
@FroggyFlox
Copy link
Member Author

FroggyFlox commented Dec 18, 2023

To do:

  • Add unit test for system.osi.replace_pattern_inline() and verify it can correctly treat instances of /
  • Update pattern used as detailed above and verify test still passes

FroggyFlox added a commit to FroggyFlox/rockstor-core that referenced this issue Dec 20, 2023
in rockstor#2558, we implemented
an automatic conversion of legacy paths to poetry bin paths for rockstor
scripts. This, however, failed for legacy paths that somehow included
double slashes.

This commit updates the pattern used to recognize this legacy paths so
that those with double slashes are also converted to poetry paths.

Includes 1 new unit test to cover underlying function.
Includes black formatting.
phillxnet added a commit that referenced this issue Dec 21, 2023
…ry-paths-after-update

Account for eventual double slahes in the conversion from legacy to poetry paths #2757
@phillxnet
Copy link
Member

Closing as:
Fixed by #2771

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants