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.

Sign up for GitHub

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

Jump to bottom

Python DownloadProgress.add_new_download() callback user data issue #1441

Open
es-fabricemarie opened this issue Apr 24, 2024 · 7 comments · May be fixed by #1849 or #1769
Open

Python DownloadProgress.add_new_download() callback user data issue #1441

es-fabricemarie opened this issue Apr 24, 2024 · 7 comments · May be fixed by #1849 or #1769
Assignees
@jrohel
Labels
Priority: MEDIUM Triaged Someone on the DNF 5 team has read the issue and determined the next steps to take

Comments

@es-fabricemarie
Copy link

Hi,

I'm trying to use DownloadProgress callbacks using the Python bindings and I face an issue with the add_new_download method.

The code to reproduce my issue can be simplified to:

#!/usr/bin/python3
import libdnf5

class DownloadProgress(libdnf5.repo.DownloadCallbacks):
    # Notify the client that a new download has been created.
    #
    # @param user_data User data entered together with url/package to download.
    # @param description The message describing new download (url/packagename).
    # @param total_to_download Total number of bytes to download.
    # @return Associated user data for new download.
    def add_new_download(self, user_data, description, total_to_download):
        print("Just added package %s to download list" % description)
        return description


base = libdnf5.base.Base()
base.load_config_from_file()
base.setup()
sack = base.get_repo_sack()
sack.create_repos_from_system_configuration()
sack.update_and_load_enabled_repos(True)
goal = libdnf5.base.Goal(base)
goal.add_rpm_upgrade()
transaction = goal.resolve()
downloader_callbacks = DownloadProgress()
base.set_download_callbacks(
    libdnf5.repo.DownloadCallbacksUniquePtr(downloader_callbacks)
)
transaction.download()

I'm trying to set the package name as the Associated user data for new download, so that it gets sent back to the other callbacks (progress and end for instance), so I can report fine grained progress.

I run this code, the script crashes and a core is dumped:

Just added package docker-buildx-plugin-0:0.14.0-1.fc39.x86_64 to download list
terminate called after throwing an instance of 'Swig::DirectorTypeMismatchException'
  what():  SWIG director type mismatch in output value of type 'void *'
Aborted (core dumped)

What am I doing wrong here? Or is this an issue with the Python binding?
@jan-kolarik
Copy link
Member

The first thing I would look into is probably implementing all the methods from the libdnf5.repo.DownloadCallbacks interface in your DownloadProgress class - there is also the progress, end, mirror_failure and fastest_mirror methods. That might be the source of an error if these methods are missing and the library tries to call them.

@es-fabricemarie
Copy link
Author

es-fabricemarie commented May 2, 2024 •
edited
Loading

I did of course. But the code above is the shortest way to reproduce my issue.
In my example, as soon as add_new_download() method returns, it dumps core.
All the examples I found return None. But when returning none, we lose the ability to pass metadata to the subsequent progress, end, etc.. callbacks.

@jan-kolarik
Copy link
Member

jan-kolarik commented May 7, 2024 •
edited
Loading

Right, I was able to reproduce the error with your code snippet. The problem lies in the return value from add_new_download. In the original C++ code, it's of type void *, which isn't currently handled by SWIG in our Python bindings. While this works with the C++ API, we need to address it in the Python bindings. In the meantime, you might find a workaround by using locally defined variables in the DownloadProgress class to store state. Check out this code example from our Python unit tests.

@jan-kolarik jan-kolarik added Triaged Someone on the DNF 5 team has read the issue and determined the next steps to take Priority: MEDIUM labels May 7, 2024
@es-fabricemarie
Copy link
Author

The example code you linked doesn't seem to allow a program to keep track of concurrent packages downloading simultaneously. This appears to only be doable with the callbacks.

I've looked at SWIG documentation on how to add typemaps in the repo.i file to support that, but I've a hit a massive high wall, the learning curve is just to steep for me at this point in time. 😰

@es-fabricemarie
Copy link
Author

@jan-kolarik let me know how I can help with this issue (short of contributing a PR), like with testing and such.

@es-fabricemarie es-fabricemarie linked a pull request Oct 10, 2024 that will close this issue
Draft
@es-fabricemarie
Copy link
Author

@jan-kolarik I hope this PR helps.

@jan-kolarik
Copy link
Member

Hi @es-fabricemarie, sorry for the late response. We've already started looking into this, will update the upstream issue here to reflect the correct state. And thanks for your related PR!

@jrohel jrohel linked a pull request Nov 11, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jrohel jrohel

Labels
Priority: MEDIUM Triaged Someone on the DNF 5 team has read the issue and determined the next steps to take
Projects
DNF team
Status: In Progress
Milestone
No milestone
3 participants
@jrohel @es-fabricemarie @jan-kolarik