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

Update pants-plugins/uses_services to support checking for redis #5893

Merged
merged 3 commits into from
Feb 9, 2023

Conversation

cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Feb 7, 2023

Background

This is another part of introducing pants, as discussed in various TSC meetings.

Related PRs can be found in:

Overview of this PR

This PR builds on #5864 and #5884 to further improve the DX (developer experience) by failing as early as possible if the development (or CI) environment does not have the required services.

This PR adds checks for redis, like #5864 added mongo checks and #5884 added rabbitmq checks.

Please see #5864 for a more detailed description of pants-plugins/uses_services.

is_redis_running.py script and the rule that runs it

is_redis_running.py is the part that checks to see if redis is running.

The pants plugin cannot import anything from our other st2* code, so is_redis_running.py is a minimal self-contained script that mirrors how st2 connects to redis. It should be as minimal as possible so that keeping it up-to-date with the core st2 code is not onerous.

The is_redis_running.py rule gets opened and run in a pex with the same rule logic as is_mongo_running.py and inspect_platform.py (see #5864).

Redis, like rabbitmq, embeds all auth stuff in the connection url. So, there are not as many settings to worry about as there were with mongo. For now, this only supports the url hard-coded in the tests. Here is where the rule defines the default url:

@dataclass(frozen=True)
class UsesRedisRequest:
"""One or more targets need a running redis service using these settings.
The coord_* attributes represent the coordination settings from st2.conf.
In st2 code, they come from:
oslo_config.cfg.CONF.coordination.url
"""
# These config opts for integration tests are in:
# conf/st2.dev.conf (copied to conf/st2.ci.conf)
# TODO: for int tests: set url by either modifying st2.{dev,ci}.conf on the fly or via env vars.
# with our version of oslo.config (newer are slower) we can't directly override opts w/ environment variables.
coord_url: str = "redis://127.0.0.1:6379"

Here is the definition of the rule that runs is_redis_running.py:

async def redis_is_running(
request: UsesRedisRequest, platform: Platform
) -> RedisIsRunning:

So, when another rule Gets RedisIsRunning with a UsesRedisRequest, pants will also run the rule that generates Platform (described in #5864), and then it will run this is_redis_running rule.

The is_redis_running rule either returns RedisIsRunning() if it is running, or raises ServiceMissingError if it is not. By raising an error, this actually breaks a convention for pants rules. Exceptions stop everything and get shown to the user right away, and for most goals, pants wants to see some dataclass returned that has something like a succeeded boolean instead. But, we want to error as early as possible, so this breaks that convention on purpose.

wiring up the test goal so it runs is_redis_running when pytest runs on a target with the uses field.

The last piece that ties this all together is a rule that makes sure the is_redis_running rule runs before pytest runs (if pants is running it on a target with the uses field). Here is the definition of the redis_is_running_for_pytest rule:

async def redis_is_running_for_pytest(
request: PytestUsesRedisRequest,
) -> PytestPluginSetup:

This rule is very simple. It just triggers running the is_redis_running rule:

# this will raise an error if redis is not running
_ = await Get(RedisIsRunning, UsesRedisRequest())
return PytestPluginSetup()

This rule needs the PytestUsesRedisRequest which selects targets that have the uses field.

class PytestUsesRedisRequest(PytestPluginSetupRequest):
@classmethod
def is_applicable(cls, target: Target) -> bool:
if not target.has_field(UsesServicesField):
return False
uses = target.get(UsesServicesField).value
return uses is not None and "redis" in uses

This request will be made by the pants-internal pytest rules thanks to this UnionRule, which is a way to declare that our class "implements" the PytestPluginSetupRequest:

UnionRule(PytestPluginSetupRequest, PytestUsesRedisRequest),

If we need to add uses support to other targets, we will need to find similar UnionRules where we can inject our rule logic. For now, I've only wired this up so our uses rules will run for pytest.

@cognifloyd cognifloyd added this to the pants milestone Feb 7, 2023
@cognifloyd cognifloyd requested review from winem, arm4b, nzlosh, rush-skills, amanda11 and a team February 7, 2023 17:26
@cognifloyd cognifloyd self-assigned this Feb 7, 2023
@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label Feb 7, 2023
@cognifloyd cognifloyd force-pushed the pants-plugins-uses_services-redis branch from 06aedfa to d31e318 Compare February 7, 2023 20:42
@cognifloyd cognifloyd force-pushed the pants-plugins-uses_services-redis branch from b37127e to a8fd0d3 Compare February 7, 2023 23:12
Copy link
Member

@rush-skills rush-skills left a comment

Choose a reason for hiding this comment

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

LGTM. Though is redis only required for orquestra tests and runner builds?

@cognifloyd
Copy link
Member Author

I don't know. They are needed at least for the orquesta tests. When we find other tests that need any of the services (mongo, rabbitmq, redis), then we can add uses= to the BUILD metadata for those tests too.

@cognifloyd cognifloyd added this pull request to the merge queue Feb 9, 2023
@cognifloyd cognifloyd removed this pull request from the merge queue due to the queue being cleared Feb 9, 2023
@cognifloyd cognifloyd merged commit eca0b2d into master Feb 9, 2023
@cognifloyd cognifloyd deleted the pants-plugins-uses_services-redis branch February 9, 2023 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure: ci/cd pantsbuild size/L PR that changes 100-499 lines. Requires some effort to review. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants