-
Notifications
You must be signed in to change notification settings - Fork 12
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 a docker-compose run --rm ci-admin
command
#44
base: main
Are you sure you want to change the base?
Conversation
This commit introduces a new docker-compose command to run the ci-admin tool. Note: If the `requirements/test.txt` file changes, the Docker image needs to be rebuilt to include the updated dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I'm not sure I'll use it myself, but it sounds like it's very useful to folks on macOS! One required change below, and another suggestion.
@@ -0,0 +1,28 @@ | |||
FROM ubuntu:latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're going to a lot of trouble to install Python here. Perhaps you should use python:3.11
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that would make it a lot simpler!
I ran into an issue using the python:3.11
image. It's related to the in-tree event_loop
fixture.
The log looks like this:
==================================== ERRORS ====================================
____________ ERROR at setup of check_default_branches_for_git_repos ____________
fixturedef = <FixtureDef argname='event_loop' scope='session' baseid=''>
@pytest.hookimpl(hookwrapper=True)
def pytest_fixture_setup(
fixturedef: FixtureDef,
) -> Generator[None, Any, None]:
"""Adjust the event loop policy when an event loop is produced."""
if fixturedef.argname == "event_loop":
# The use of a fixture finalizer is preferred over the
# pytest_fixture_post_finalizer hook. The fixture finalizer is invoked once
# for each fixture, whereas the hook may be invoked multiple times for
# any specific fixture.
# see https://github.com/pytest-dev/pytest/issues/5848
_add_finalizers(
fixturedef,
_close_event_loop,
_restore_event_loop_policy(asyncio.get_event_loop_policy()),
_provide_clean_event_loop,
)
outcome = yield
loop: asyncio.AbstractEventLoop = outcome.get_result()
# Weird behavior was observed when checking for an attribute of FixtureDef.func
# Instead, we now check for a special attribute of the returned event loop
fixture_filename = inspect.getsourcefile(fixturedef.func)
if not getattr(loop, "__original_fixture_loop", False):
> _, fixture_line_number = inspect.getsourcelines(fixturedef.func)
/usr/local/lib/python3.11/site-packages/pytest_asyncio/plugin.py:756:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
/usr/local/lib/python3.11/inspect.py:1240: in getsourcelines
lines, lnum = findsource(object)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
object = <function event_loop at 0xffff9511e980>
def findsource(object):
"""Return the entire source file and starting line number for an object.
The argument may be a module, class, method, function, traceback, frame,
or code object. The source code is returned as a list of all the lines
in the file and the line number indexes a line in that list. An OSError
is raised if the source code cannot be retrieved."""
file = getsourcefile(object)
if file:
# Invalidate cache if needed.
linecache.checkcache(file)
else:
file = getfile(object)
# Allow filenames in form of "<something>" to pass through.
# doctest monkeypatches linecache module to enable
# inspection, so let linecache.getlines to be called.
if not (file.startswith('<') and file.endswith('>')):
raise OSError('source code not available')
module = getmodule(object, file)
if module:
lines = linecache.getlines(file, module.__dict__)
else:
lines = linecache.getlines(file)
if not lines:
> raise OSError('could not get source code')
E OSError: could not get source code
/usr/local/lib/python3.11/inspect.py:1077: OSError
=============================== warnings summary ===============================
check_default_branches.py::check_default_branches_for_git_repos
/usr/local/lib/python3.11/site-packages/_pytest/fixtures.py:1069: PluggyTeardownRaisedWarning: A plugin raised an exception during an old-style hookwrapper teardown.
Plugin: asyncio, Hook: pytest_fixture_setup
OSError: could not get source code
For more information see https://pluggy.readthedocs.io/en/stable/api_reference.html#pluggy.PluggyTeardownRaisedWarning
result = ihook.pytest_fixture_setup(fixturedef=self, request=request)
check_worker_pools.py::check_gcp_ssds
/usr/local/lib/python3.11/site-packages/pytest_asyncio/plugin.py:814: DeprecationWarning: pytest-asyncio detected an unclosed event loop when tearing down the event_loop
fixture: <_UnixSelectorEventLoop running=False closed=False debug=False>
pytest-asyncio will close the event loop for you, but future versions of the
library will no longer do so. In order to ensure compatibility with future
versions, please make sure that:
1. Any custom "event_loop" fixture properly closes the loop after yielding it
2. The scopes of your custom "event_loop" fixtures do not overlap
3. Your code does not modify the event loop in async fixtures or tests
warnings.warn(
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================== short test summary info ============================
ERROR src/ciadmin/check/check_default_branches.py::check_default_branches_for_git_repos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* squints * maybe its this
pytest-asyncio will close the event loop for you, but future versions of the
library will no longer do so. In order to ensure compatibility with future
versions, please make sure that:
1. Any custom "event_loop" fixture properly closes the loop after yielding it
2. The scopes of your custom "event_loop" fixtures do not overlap
3. Your code does not modify the event loop in async fixtures or tests
services: | ||
ci-admin: | ||
build: . | ||
volumes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make it possible to forward TASKCLUSTER_ACCESS_TOKEN
and TASKCLUSTER_CLIENT_ID
here. Without them, you can't run apply
(if it's ever needed), or even diff
, which requires limited scopes.
What if instead of checking this in, we added a one-liner to the README that can be copy/pasted? If you use the python images like Ben suggested, there isn't really anything to do other than install dependencies. Which would be easy enough to hardcode into the Alternatively, maybe there could be a task that builds this image automatically and stores it in the Github image registry? |
I could be convinced that we shouldn't do this, but I also don't think it's a bad thing to have a cross platform way to run things (as we do for many of our other projects). I know that running Linux-targeted stuff on macOS is often a pain, even when it's Python based.
One downside to this is that you won't be able to use the prebuilt image if you're changing ciadmin code, I think? If the image being built is done so "taskcluster-style" (that is, it uses If we do end up taking this, I would really like to run down the issues that are preventing the |
This commit introduces a new docker-compose command to run the ci-admin tool.
Note: If the
requirements/test.txt
file changes, the Docker image needs to be rebuilt to include the updated dependencies.Example: