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

Add support for Galaxy v3 API #45

Merged
merged 6 commits into from
Apr 14, 2023

Conversation

felixfontein
Copy link
Collaborator

Tested by building docs (with antsibull-docs) from a collection on Galaxy where galaxy_url = "https://beta-galaxy.ansible.com" is in antsibull-docs.cfg.

@felixfontein felixfontein requested a review from gotmax23 April 8, 2023 13:11
@felixfontein
Copy link
Collaborator Author

CC @samccann

@felixfontein
Copy link
Collaborator Author

Turns out the v3 API of beta-galaxy.ansible.com is incredibly slow. A simple curl -i 'https://beta-galaxy.ansible.com/api/v3/plugin/ansible/content/published/collections/index/community/general/versions/?limit=50' takes 30 seconds for me, and that's just the first page (out of three at the moment).

The Galaxy v3 API versions list test is disabled since it is so incredibly
slow. Just listing the first page takes 30 seconds (!). The total listing
takes > 100 seconds.
@@ -7,9 +7,11 @@

from __future__ import annotations

import asyncio
import os.path
import shutil
import typing as t
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's some instances of old type hinting syntax. Do you have a reason for keeping those or should I point them all out?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main reason is that this needs to be backported to stable-1, so it's easier to keep them and remove them later (after backporting). Otherwise I have to do the work twice.

src/antsibull_core/galaxy.py Show resolved Hide resolved
src/antsibull_core/galaxy.py Show resolved Hide resolved
src/antsibull_core/galaxy.py Show resolved Hide resolved
cache_path = tmp_path_factory.mktemp('cache')
downloader = CollectionDownloader(
aio_session,
str(download_path),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: we should change this API to accept PathLike objects as well in a separate PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ea50a8d.

Comment on lines +59 to +67
class GalaxyContext:
server: str
version: GalaxyVersion
base_url: str

def __init__(self, server: str, version: GalaxyVersion, base_url: str) -> None:
self.server = server
self.version = version
self.base_url = base_url
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a good candidate for a dataclass. After adding import dataclasses:

Suggested change
class GalaxyContext:
server: str
version: GalaxyVersion
base_url: str
def __init__(self, server: str, version: GalaxyVersion, base_url: str) -> None:
self.server = server
self.version = version
self.base_url = base_url
@dataclasses.dataclass
class GalaxyContext:
server: str
version: GalaxyVersion
base_url: str

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is 3.7+, so not backportable. We can do this in a follow-up PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack. For what it's worth, there's a backport on PyPI that we could add for python_version<'3.7', but I don't think that's worth it.

src/antsibull_core/galaxy.py Show resolved Hide resolved
@felixfontein felixfontein added the backport-1 Automatically create backport to stable-1 branch label Apr 9, 2023
Copy link
Collaborator

@gotmax23 gotmax23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack. Thanks!

Comment on lines +59 to +67
class GalaxyContext:
server: str
version: GalaxyVersion
base_url: str

def __init__(self, server: str, version: GalaxyVersion, base_url: str) -> None:
self.server = server
self.version = version
self.base_url = base_url
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack. For what it's worth, there's a backport on PyPI that we could add for python_version<'3.7', but I don't think that's worth it.

return cls(galaxy_server, version, base_url)


_GALAXY_CONTEXT_CACHE: dict[str, t.Union[GalaxyContext, asyncio.Future]] = {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need to be changed to t.Dict when backporting.

@samccann
Copy link

I forgot to mention this before but it's a known issue that the galaxy beta site is slow. That's one reason they haven't broadcast the announcement yet of things switching over. They are working on improving the performance for sure.

@felixfontein
Copy link
Collaborator Author

The curl request from above is down to 10 seconds now, BTW. Still way too slow, but hey, three times faster than a week ago...

@felixfontein felixfontein merged commit af8f28d into ansible-community:main Apr 14, 2023
@patchback
Copy link

patchback bot commented Apr 14, 2023

Backport to stable-1: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply af8f28d on top of patchback/backports/stable-1/af8f28d394c63996909c8c7f1caa7f78a7994dc6/pr-45

Backporting merged PR #45 into main

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. Make sure you have an upstream repo added as a remote too. In these
    instructions you'll refer to it by the name upstream. If you don't
    have it, here's how you can add it:
    $ git remote add upstream https://github.com/ansible-community/antsibull-core.git
  3. Ensure you have the latest copy of upstream and prepare a branch
    that will hold the backported code:
    $ git fetch upstream
    $ git checkout -b patchback/backports/stable-1/af8f28d394c63996909c8c7f1caa7f78a7994dc6/pr-45 upstream/stable-1
  4. Now, cherry-pick PR Add support for Galaxy v3 API #45 contents into that branch:
    $ git cherry-pick -x af8f28d394c63996909c8c7f1caa7f78a7994dc6
    If it'll yell at you with something like fatal: Commit af8f28d394c63996909c8c7f1caa7f78a7994dc6 is a merge but no -m option was given., add -m 1 as follows instead:
    $ git cherry-pick -m1 -x af8f28d394c63996909c8c7f1caa7f78a7994dc6
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR Add support for Galaxy v3 API #45 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub:
    $ git push origin patchback/backports/stable-1/af8f28d394c63996909c8c7f1caa7f78a7994dc6/pr-45
  7. Create a PR, ensure that the CI is green. If it's not — update it so that
    the tests and any other checks pass. This is it!
    Now relax and wait for the maintainers to process your pull request
    when they have some cycles to do reviews. Don't worry — they'll tell you if
    any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein felixfontein deleted the galaxy-v3 branch April 14, 2023 04:40
@felixfontein
Copy link
Collaborator Author

@gotmax23 thanks for reviewing this!

felixfontein added a commit to felixfontein/antsibull-core that referenced this pull request Apr 14, 2023
* Add support for Galaxy v3 API.

* Add integration tests for the Galaxy API.

The Galaxy v3 API versions list test is disabled since it is so incredibly
slow. Just listing the first page takes 30 seconds (!). The total listing
takes > 100 seconds.

* Reorganize imports.

* Improve docstrings; validate context.server == galaxy_server.

* Fix check, update docstring.

* According to ansible-galaxy CLI code, the result value can sometimes be called 'results' also for v3.

(cherry picked from commit af8f28d)
felixfontein added a commit to felixfontein/antsibull-core that referenced this pull request Apr 14, 2023
* Add support for Galaxy v3 API.

* Add integration tests for the Galaxy API.

The Galaxy v3 API versions list test is disabled since it is so incredibly
slow. Just listing the first page takes 30 seconds (!). The total listing
takes > 100 seconds.

* Reorganize imports.

* Improve docstrings; validate context.server == galaxy_server.

* Fix check, update docstring.

* According to ansible-galaxy CLI code, the result value can sometimes be called 'results' also for v3.

(cherry picked from commit af8f28d)
gotmax23 pushed a commit that referenced this pull request Apr 14, 2023
* Add support for Galaxy v3 API (#45)

* Add support for Galaxy v3 API.

* Add integration tests for the Galaxy API.

The Galaxy v3 API versions list test is disabled since it is so incredibly
slow. Just listing the first page takes 30 seconds (!). The total listing
takes > 100 seconds.

* Reorganize imports.

* Improve docstrings; validate context.server == galaxy_server.

* Fix check, update docstring.

* According to ansible-galaxy CLI code, the result value can sometimes be called 'results' also for v3.

(cherry picked from commit af8f28d)

* Make work with older environments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-1 Automatically create backport to stable-1 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants