Comments (29)
That said, if the interim solution is within acceptable reach (it doesn't take you hours of work), I'll be happy to assist with the debugging <3
I'll see what we can do, although dont hold your breath for v2.0 because I suspect this change might also break a lot of mamba python tests .
As an aside question: should we be initializing the libmamba context ourselves? I was surprised to see that I still needed to set the context bool for the signal handlers, so I don't know if it's still being used behind the scenes for something else.
In C++ you have to construct yourself the context (with whatever options you want) and pass it to other functions afterwards.
In python, libmambapy will create itself the context and provide it transparently, and this is mandatory as long as we want to keep the interface as before for python. If we decide we want to break it for v2.0 then you will have to change code using libmamba to create a context yourself and pass it to some other functions. While it does make more sense that way, it's also breaking a lot of interface.
from mamba.
This pr should make sure that the test script will work with KeyboardInterrupt
(it works at least for me on Windows)
It will need some testing.
Other than that we agreed with others at Quantstack to go with just breaking the interface and see after if we need to provide help or an api layer for helping with current users integration. The first step is to just make available in python the creation of the Context
, then make it mandatory argument for functions that does need it in C++. This will taker time because the python tests needs to be updated accordingly. Then we'll see if we want to make transparent the context argument (by defaulting it in some ways).
from mamba.
Cool, I'll wait for the next beta! Thanks again!
from mamba.
@jaimergp The beta is ready, could you try the workaround Context
construction and tell me if it's enough for your use case? https://github.com/mamba-org/mamba/releases/tag/2024.06.14
from mamba.
Yeah it makes sense, it would require #3297 to make it work as expected (KeyboardInterrupt
) with the first Ctrl+C, but I couldnt merge this yet because it only works on windows. The linux-specific code is a problem we took the time to analyze, we have a solution designed but it will take time to come and probably after the release.
Meanwhile, I guess we could solve at least the windows case by merging it. What do you think @JohanMabille
from mamba.
With #3329 I can make this work locally on Linux and Windows (Ctrl+C triggers a KeyboardInterrupt
):
import time
from libmambapy import Context, ContextOptions
mamba_context = Context(ContextOptions(enable_logging = True, enable_signal_handling = False))
print("Sleep for 10s... Try to Ctrl+C")
time.sleep(10)
from mamba.
#3329 is now merged, we just need #3297 and we're good for testing that workaround.
from mamba.
Now #3297 is merged and we plan to make a release soon, hopefully this will be easier to work with @jaimergp
from mamba.
Thanks @Klaim, I gave it a try and now I do get the KeyboardInterrupt
... eventually. For some reason the .solv
parsing is super slow:
DEBUG conda.conda_libmamba_solver.index:_load_repo_info_from_json_path(395): Loading conda-forge (https://conda.anaconda.org/conda-forge/win-64) from SOLV repodav
[2024-07-08 14:44:29.258] [info] [output.cpp:600] Attempting to read libsolv solv file "C:\\Users\\JaimeRodriguez-Guerr\\Documents\\GitHub\\conda\\devenv\\Window4
[2024-07-08 14:44:29.258] [info] [output.cpp:600] Expecting solv metadata : {"etag":"\"37a041dcbbafee7a2420abe9055a63e1\"","mod":"Mon, 08 Jul 2024 11:58:54 GMT",}
[2024-07-08 14:45:02.826] [info] [output.cpp:600] Loaded solv metadata : {"etag":"\"37a041dcbbafee7a2420abe9055a63e1\"","mod":"Mon, 08 Jul 2024 11:58:54 GMT","to}
[2024-07-08 14:45:02.827] [info] [output.cpp:600] Metadata from solv are valid, loading successful
~30s for conda-forge on win-64. This call is blocking so it takes a while for Python to recover control to raise the KeyboardInterrupt. I don't know if this performance is a regression but my experience in Unix systems is that .solv loading is almost instantaneous.
edit: Turns out SOLV files are known to be very slow on Windows so we are better off without them (src: #2753 (comment)). That's ok with me.
from mamba.
Is this issue also happening with alpha4
version? or just beta
?
from mamba.
You are right, this works correctly on alpha4
. At least on Linux :)
Sorry, I tested the wrong file. I can reproduce the original report with alpha4 too. IOW, Ctrl+C doesn't work there either.
from mamba.
Thanks for #3285 and the beta1 release. Unfortunately, it still doesn't fix the reproducer script for me. IOW, this still gets hung:
import time
from libmambapy import Context
# This line below doesn't have any effect;
# it can be commented out and the result is the same
Context.use_default_signal_handler(False)
print("Sleep for 10s... Try to Ctrl+C")
time.sleep(10)
With versions:
libmamba 2.0.0beta1 h19be64c_0 conda-forge/label/mamba_dev
libmambapy 2.0.0beta1 py311h1564048_0 conda-forge/label/mamba_dev
Is that how I am supposed to use it? 🤔
Additionally, is #3285 accounting for Windows too?
from mamba.
Yes that's the way to use it, so it is still broken :s
from mamba.
I'm a bit surprised as it was working well for me but now I see that it only works when doing Ctrl+C
twice. Not sure why, but this hotfix was part of a bigger fix so probably I didnt integrate something from the other fix (which was not complete yet).
I'm on it.
from mamba.
Correction: I was trying the wrong branch.
The branch from the pr seems to work for me, one Ctrl+C does break the sleep. (I was surprised before because I used the same test script).
I also tested on main
and it still works.
@jaimergp Looks like something is different between our testing cases, could you give more info about your setup so that I can attempt to reproduce it?
To clarify: I'm testing on Windows right now.
from mamba.
I'm on macOS and also tested on Linux via Docker:
$ conda create -n debug-libmamba-v2b1-ctrl-c --override-channels -c conda-forge/label/mamba_dev -c conda-forge python=3.10 libmambapy
Channels:
- conda-forge/label/mamba_dev
- conda-forge
Platform: osx-arm64
Collecting package metadata (repodata.json): done
Solving environment: done
==> WARNING: A newer version of conda exists. <==
current version: 24.1.1
latest version: 24.5.0
Please update conda by running
$ conda update -n base -c conda-forge conda
## Package Plan ##
environment location: /Users/jrodriguez/.local/anaconda/envs/debug-libmamba-v2b1-ctrl-c
added / updated specs:
- libmambapy
- python=3.10
The following packages will be downloaded:
package | build
---------------------------|-----------------
libmambapy-2.0.0beta1 | py310h7ae444f_0 527 KB conda-forge/label/mamba_dev
------------------------------------------------------------
Total: 527 KB
The following NEW packages will be INSTALLED:
bzip2 conda-forge/osx-arm64::bzip2-1.0.8-h93a5062_5
c-ares conda-forge/osx-arm64::c-ares-1.28.1-h93a5062_0
ca-certificates conda-forge/osx-arm64::ca-certificates-2024.2.2-hf0a4a13_0
cpp-expected conda-forge/osx-arm64::cpp-expected-1.1.0-hffc8910_0
fmt conda-forge/osx-arm64::fmt-10.2.1-h2ffa867_0
icu conda-forge/osx-arm64::icu-73.2-hc8870d7_0
krb5 conda-forge/osx-arm64::krb5-1.21.2-h92f50d5_0
libarchive conda-forge/osx-arm64::libarchive-3.7.2-hcacb583_1
libcurl conda-forge/osx-arm64::libcurl-8.7.1-h2d989ff_0
libcxx conda-forge/osx-arm64::libcxx-17.0.6-h5f092b4_0
libedit conda-forge/osx-arm64::libedit-3.1.20191231-hc8eb9b7_2
libev conda-forge/osx-arm64::libev-4.33-h93a5062_2
libffi conda-forge/osx-arm64::libffi-3.4.2-h3422bc3_5
libiconv conda-forge/osx-arm64::libiconv-1.17-h0d3ecfb_2
libmamba conda-forge/label/mamba_dev/osx-arm64::libmamba-2.0.0beta1-h2db0620_0
libmambapy conda-forge/label/mamba_dev/osx-arm64::libmambapy-2.0.0beta1-py310h7ae444f_0
libnghttp2 conda-forge/osx-arm64::libnghttp2-1.58.0-ha4dd798_1
libsolv conda-forge/osx-arm64::libsolv-0.7.29-h1efcc80_0
libsqlite conda-forge/osx-arm64::libsqlite-3.45.3-h091b4b1_0
libssh2 conda-forge/osx-arm64::libssh2-1.11.0-h7a5bd25_0
libxml2 conda-forge/osx-arm64::libxml2-2.12.7-ha661575_0
libzlib conda-forge/osx-arm64::libzlib-1.2.13-h53f4e23_5
lz4-c conda-forge/osx-arm64::lz4-c-1.9.4-hb7217d7_0
lzo conda-forge/osx-arm64::lzo-2.10-h93a5062_1001
ncurses conda-forge/osx-arm64::ncurses-6.5-hb89a1cb_0
nlohmann_json conda-forge/osx-arm64::nlohmann_json-3.11.3-hebf3989_0
openssl conda-forge/osx-arm64::openssl-3.3.0-h0d3ecfb_0
pybind11-abi conda-forge/noarch::pybind11-abi-4-hd8ed1ab_3
python conda-forge/osx-arm64::python-3.10.14-h2469fbe_0_cpython
python_abi conda-forge/osx-arm64::python_abi-3.10-4_cp310
readline conda-forge/osx-arm64::readline-8.2-h92ec313_1
reproc conda-forge/osx-arm64::reproc-14.2.4.post0-h93a5062_1
reproc-cpp conda-forge/osx-arm64::reproc-cpp-14.2.4.post0-h965bd2d_1
simdjson conda-forge/osx-arm64::simdjson-3.9.2-h420ef59_0
spdlog conda-forge/osx-arm64::spdlog-1.12.0-he64bfa9_2
tk conda-forge/osx-arm64::tk-8.6.13-h5083fa2_1
tzdata conda-forge/noarch::tzdata-2024a-h0c530f3_0
xz conda-forge/osx-arm64::xz-5.2.6-h57fd34a_0
yaml-cpp conda-forge/osx-arm64::yaml-cpp-0.8.0-h13dd4ca_0
zstd conda-forge/osx-arm64::zstd-1.5.6-hb46c0d2_0
Proceed ([y]/n)? y
Downloading and Extracting Packages:
Preparing transaction: done
Verifying transaction: done
Executing transaction: done
#
# To activate this environment, use
#
# $ conda activate debug-libmamba-v2b1-ctrl-c
#
# To deactivate an active environment, use
#
# $ conda deactivate
$ conda activate debug-libmamba-v2b1-ctrl-c
$ python ~/devel/conda-libmamba-solver/debug_ctrl_c.py
libmambapy version 2.0.0
Sleep for 5s... Try to Ctrl+C
^C^C^C^C^C^C^C^C^C^C^C%
$ conda info
active environment : debug-libmamba-v2b1-ctrl-c
active env location : /Users/jrodriguez/.local/anaconda/envs/debug-libmamba-v2b1-ctrl-c
shell level : 2
user config file : /Users/jrodriguez/.condarc
populated config files : /Users/jrodriguez/.local/anaconda/.condarc
/Users/jrodriguez/.condarc
conda version : 24.1.1
conda-build version : 24.1.2
python version : 3.9.9.final.0
solver : libmamba (default)
virtual packages : __archspec=1=m1
__conda=24.1.1=0
__osx=14.3.1=0
__unix=0=0
base environment : /Users/jrodriguez/.local/anaconda (writable)
conda av data dir : /Users/jrodriguez/.local/anaconda/etc/conda
conda av metadata url : None
channel URLs : https://conda.anaconda.org/conda-forge/osx-arm64
https://conda.anaconda.org/conda-forge/noarch
package cache : /Users/jrodriguez/.local/anaconda/pkgs
/Users/jrodriguez/.conda/pkgs
envs directories : /Users/jrodriguez/.local/anaconda/envs
/Users/jrodriguez/.conda/envs
platform : osx-arm64
user-agent : conda/24.1.1 requests/2.31.0 CPython/3.9.9 Darwin/23.3.0 OSX/14.3.1 solver/libmamba conda-libmamba-solver/24.1.0 libmambapy/1.5.6
UID:GID : 501:20
netrc file : None
offline mode : False
I haven't tested the beta1 on Windows, but what I observed there during the early testing is that the process is finished after a couple of Ctrl+C, but abruptly. The correct behavior is that Python catches the signal and raises KeyboardInterrupt
, which should appear in the output.
from mamba.
It's possible that the hotfix is replacing a handler introduced by python, I'll try to do that better.
from mamba.
Is it possible to have the C++ signal handler be opt-in via the application layer? IOW, that micromamba sets the handler explicitly, but that otherwise there's no signal specific logic set in libmamba(py)?
from mamba.
Currently using libmamba
in C++ will not set the handler by default and the option can be set through the Context
constructor, see:
As you can see the signal handling is not set by default, but this is set to true
in mamba
and micromamba
.
The issue here, indeed, is that in libmambapy we didnt want to break the way you use the library in python, so we had to default true
for that option to keep the previous behavior (note that the issue here became apparent after the Context became not-a-singleton).
The rigth solution to avoid this would be to change the python API so that the context has to be manually constructed with options exactly like in C++. We planned to do that in a future release though.
Meanwhile I'm trying to fix the issue without changing that API for the immediate time, although if you think you can wait for a future version I can stop and focus on the bigger better solution but it will only be available in a future minor release.
As for the solution I'm trying: keep track of the previous handler that had been set and reuse it either when we set the option to false and/or in the handler we set.
from mamba.
Thanks for the details!
v2 contains plenty of API breaking changes already so one more would be acceptable and not a problem. If you'd rather go that way, I'm fine waiting for that and sticking to v1 for the time being. Actually I think we are not explicitly initializing the context in conda/conda-libmamba-solver#457 anymore because we explicitly configure the Database and Solver with all the options (we do take the default values from conda's context, not libmamba's).
That said, if the interim solution is within acceptable reach (it doesn't take you hours of work), I'll be happy to assist with the debugging <3
As an aside question: should we be initializing the libmamba context ourselves? I was surprised to see that I still needed to set the context bool for the signal handlers, so I don't know if it's still being used behind the scenes for something else.
from mamba.
Some update for this issue:
- #3309 Will require changing python scripts to allow explicit creation of the
Context
object. Once created, all the internals of the module will be ready to use, but before that point they will not be usable. I am in the process of also changing the functions which require access toContext
so that it doesnt do it as a singleton internally. Although, for now, because of technical limitations, we will still prevent creating more than oneContext
(the limitation doesnt exist in C++ except if you start to usemamba::Console
andmamba::Configuration
... so lets keep it simple in Python for now.
Currently with this branch on my linux and windows I can do:
import time
from libmambapy import Context, ContextOptions
mamba_context = Context()
print("Sleep for 10s... Try to Ctrl+C")
time.sleep(10)
In this case we get the default normal behavior where mamba handles the signals, which is the issue here. So now the workaround would be:
import time
from libmambapy import Context, ContextOptions
mamba_context = Context(ContextOptions(enable_logging_and_signal_handling = False))
print("Sleep for 10s... Try to Ctrl+C")
time.sleep(10)
Which prevents mamba from even starting to take the signal handling. Ctrl+C does raise a KeyboardInterrupt
as expected.
This is not enough to close this issue though.
-
#3297 is still broken on Linux, I didnt manage to fix it yet but I had to focus on point 1 , so I'll get back on it next week. If I manage to make it work then the previously proposed script will also work as expacted (in that branch, the widnows version works as expected).
-
While investigating I found that on Linux we reset (without checking options) the signal handling mechanism even if it was not enabled before in the code handling file locking waits. This part is quite tricky and to be solved requires a rewrite of the signal handling altogether (I tried several variations of a quickfix but my conclusion, after double checking with @JohanMabille that my understanding was correct, is that it's fundamentally broken as-is and needs to be a bit different).
So basically going with that rewrite (or partial rewrite) could also solve point 2.
The rewrite would be to have a clear separation beween an interface allowing to know when a special event occurs, with a callback system + storing state or something similar - what we have is close to this but not exactly), and how the event is implemented to detect (here signal handling for example. Because different parts of the code wants to know if we have been interrupted, but these parts of the code also try to do things related to how the detection is being implemented. So essnetially this needs to be better organized and that should help fix the issue (any source of interruption becomes an interruption for whatever is interested in interruptions, nothing more complicated).
from mamba.
Thanks for the explanation @Klaim! So, in short, we still need to wait a bit for the rewrite to fully address this, right?
While investigating I found that on Linux we reset (without checking options) the signal handling mechanism even if it was not enabled before in the code handling file locking waits.
Where in the code is that happening? Because maybe we are lucky in conda-libmamba-solver and we are not invoking that part (e.g. networking).
from mamba.
So, in short, we still need to wait a bit for the rewrite to fully address this, right?
We'll see after I'm done with the python api changes and the fix of on linux, we need to evaluate if it would be long to rewrite but I might do something that relies on part of the current code. Not sure yet.
Where in the code is that happening?
There: https://github.com/mamba-org/mamba/blob/main/libmamba/src/core/util.cpp#L877
This is called every time we finish locking a file on non-windows.
from mamba.
Hm, the only IO we rely on is writing the SOLV file. If we are lucky and that function is not called there... maaaybe it works?
from mamba.
You'll have to check if Ctrl+C still works after you set mamba_context = Context(ContextOptions(enable_logging_and_signal_handling = False))
and then do something, yeah
from mamba.
Hi! I checked and it works now. I observed an interesting interaction with the logging system, though.
If I initialize the context like this:
libmamba_context = libmambapy.Context(
libmambapy.ContextOptions(enable_logging_and_signal_handling=False)
)
Ctrl+C works, but I also get verbose logging to STDOUT (not STDERR). This breaks JSON output even if I cancel it via libmambapy.cancel_json_output(libmamba_context)
If I initialize without ContextOptions
, then Ctrl+C still works but the logging problem is gone. Not sure how these are related or if I'll run into problems in some edge cases if I don't pass ContextOptions
.
from mamba.
Ok thanks for testing!
I'll check why the logging outputs when not enabled but meanwhile I think splitting the option in two so that you can disable signal handling without disabling logging seems to become important. I wanted to do that at some point, now seems to be the right time.
If I initialize without ContextOptions, then Ctrl+C still works but the logging problem is gone.
Hmm that is werid, maybe I did something wrong with the defaults...
if I'll run into problems in some edge cases if I don't pass ContextOptions.
No, when Context()
is built without a ContextOptions()
with just pass a default with the option set to true
, so I'm surprised that Ctrl + C works in that case. Anyway it should just change the option's value, no other edge cases.
from mamba.
The CI ran fully and now I can also see that the tests for Windows do not pass for Ctrl+C
. Linux and macOS are ok.
And speaking of logging, I see way too many lines like these:
debug libmamba No signatures available for '21cmfast-3.3.1-py310h19eaa8d_0.conda'. Downloading without verifying artifacts.
debug libmamba Adding package record to repo 21cmfast-3.3.1-py310h19eaa8d_0.conda
debug libmamba No signatures available for '21cmfast-3.3.1-py310h19eaa8d_1.conda'. Downloading without verifying artifacts.
debug libmamba Adding package record to repo 21cmfast-3.3.1-py310h19eaa8d_1.conda
debug libmamba No signatures available for '21cmfast-3.3.1-py311hc701e3d_0.conda'. Downloading without verifying artifacts.
debug libmamba Adding package record to repo 21cmfast-3.3.1-py311hc701e3d_0.conda
debug libmamba No signatures available for '21cmfast-3.3.1-py311hc701e3d_1.conda'. Downloading without verifying artifacts.
debug libmamba Adding package record to repo 21cmfast-3.3.1-py311hc701e3d_1.conda
debug libmamba No signatures available for '21cmfast-3.3.1-py38h0db86a8_0.conda'. Downloading without verifying artifacts.
debug libmamba Adding package record to repo 21cmfast-3.3.1-py38h0db86a8_0.conda
debug libmamba No signatures available for '21cmfast-3.3.1-py38h0db86a8_1.conda'. Downloading without verifying artifacts.
debug libmamba Adding package record to repo 21cmfast-3.3.1-py38h0db86a8_1.conda
debug libmamba No signatures available for '21cmfast-3.3.1-py39h72d3284_0.conda'. Downloading without verifying artifacts.
debug libmamba Adding package record to repo 21cmfast-3.3.1-py39h72d3284_0.conda
debug libmamba No signatures available for '21cmfast-3.3.1-py39h72d3284_1.conda'. Downloading without verifying artifacts.
debug libmamba Adding package record to repo 21cmfast-3.3.1-py39h72d3284_1.conda
Basically one for each artifact. I think that's way too verbose, but I'll open a separate issue for that.
from mamba.
The CI ran fully and now I can also see that the tests for Windows do not pass for Ctrl+C.
Ok, I looked into it and it actually passes... sort of. I need to send two Ctrl+C signals and then the processed is killed immediately. However, the signal is not propagated back to Python so we don't see the KeyboardInterrupt
exception being printed or any other indication that Python handled the exception. See this commit.
from mamba.
Related Issues (20)
- "micromamba run" does not forward exit code HOT 2
- `enum class` underlying types conversions
- `--force-reinstall` doesn't seem to work after pip uninstalling
- Micromamba parses condarc YAML with <CR> line breaks incorrectly HOT 1
- Bot failure
- ENH: Allow serialization of `Context()`
- System's %PATH% is overwrite, micromamba for windows,
- Bot failure HOT 1
- When following initialization help output, getting: `The following arguments were not expected: zsh --shell`
- [Mamba 2.0] Update doc
- [Mamba 2.0] Missing profile.d script HOT 3
- Bot failure HOT 1
- Logging callbacks have too much overhead on libmambapy HOT 3
- Multi-component libmamba(py)?
- Irrelevant output
- Upgrading an environment to libgcc(-ng) 14.1.0 build 1 results in missing shared object files HOT 5
- Mamba 2.0 - installing in the base env without activation HOT 2
- Deleting ~/.cache breaks `micromamba install` HOT 2
- No Hardlinks upon environment clone and update / during Docker build
- Avoid downloading large package metadata HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from mamba.