Incosistent error for CAPABILITY_REGEX check: hyphens or underscores?

[marshallmcdonnell]

In 0.9.0, for a service we are trying to create called intersect-chess-instrument-control-service, we are getting an error about the capability name.

> CRITICAL:intersect-sdk:Invalid intersect_sdk_capability_name on capability ChessInstrumentControlCapability - must be a non-empty string with only alphanumeric characters and hyphens (you must explicitly set this, and do so on the class and not an instance).

The line that violated this check was intersect_sdk_capability_name=chess-instrument-control

Yet, there are hyphens in the name, which by the error message should work.

That error is defined here: https://github.com/INTERSECT-SDK/python-sdk/blob/main/src/intersect_sdk/_internal/schema.py#L614-L628

The capabilitiy regex is defined here: https://github.com/INTERSECT-SDK/python-sdk/blob/v0.9.0/src/intersect_sdk/constants.py#L12

The regex shows ^[a-zA-Z0-9]\w*$
According to this, the \w is the issue.
Docs: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Regular_expressions/Character_class_escape

Mainly, udderscores are allowed by \w but hyphens are not.

Simple fix for the capability name is change from chess-instrument-control to chess_instrument_control.

Yet, do we want to allow hyphens instead?

====

From issue in this intersect service: nsdf-fabric/intersect-chess-instrument-control-service#4 (comment)

@k1nho was reviewing PR, found different behavior between 0.8.3 and 0.9 versions of SDK, see below

I am running the compose setup and ran into this error for the version defined of intersect-sdk

CRITICAL:intersect-sdk:Invalid intersect_sdk_capability_name on capability ChessInstrumentControlCapability - must be a non-empty string with only alphanumeric characters and hyphens (you must explicitly set this, and do so on the class and not an instance).

After changing to intersect-sdk 0.8.3 the one we used previously, I got a better error

CRITICAL:intersect-sdk:On capability 'ChessInstrumentControlCapability', parameter 'config' type annotation 'ExperimentConfig' on function 'initialize_experiment' is in
valid TypeAdapter[ExperimentConfig] is not fully defined; you should define ExperimentConfig and all referenced types, then call .rebuild() on the instance.

For further information visit https://errors.pydantic.dev/2.12/u/class-not-fully-defined

It seems that there is some error that has to do with TypeAdapter, and that is caused by the from future import annotations on the service.py, data_models.py and file_writer.py. After removing them the intersect service ran correctly (intersect-sdk0.8.3, 0.9.0 still fails with the first error). Is there a reason why those are needed?

[Lance-Drane]

Simple fix for the capability name is change from chess-instrument-control to chess_instrument_control.
Yet, do we want to allow hyphens instead?

Allowing underscores in the capability is not an error. I had considered allowing hyphens but didn't want to allow people to make two separate capabilities like capability_one and capability-one. However, capabilities are always namespaced to a Service and not an ecosystem, so hyphens could potentially be permitted.

The reason for prohibiting brackets in Services was actually related to s3 bucket naming rules, which MINIO follows:

Bucket names can consist only of lowercase letters, numbers, periods (.), and hyphens (-)

---

For the other error (I do not have access to that repo so can't look at it further):

It seems that there is some error that has to do with TypeAdapter, and that is caused by the from future import annotations on the service.py, data_models.py and file_writer.py. After removing them the intersect service ran correctly (intersect-sdk0.8.3, 0.9.0 still fails with the first error). Is there a reason why those are needed?

The __future__ annotations was for Python 3.8 and 3.9 support, but we don't have that anymore in 0.9.0 . A significant reason why I wanted to make the minimum Python version 3.10 was to avoid dealing with __future__ imports precisely because they can cause subtle bugs like this.

[Lance-Drane]

Simple fix for the capability name is change from chess-instrument-control to chess_instrument_control.
Yet, do we want to allow hyphens instead?

Allowing underscores in the capability is not an error. I had considered allowing hyphens but didn't want to allow people to make two separate capabilities like capability_one and capability-one. However, capabilities are always namespaced to a Service and not an ecosystem, so hyphens could potentially be permitted.

The reason for prohibiting brackets in Services was actually related to s3 bucket naming rules, which MINIO follows:

Bucket names can consist only of lowercase letters, numbers, periods (.), and hyphens (-)

For the other error (I do not have access to that repo so can't look at it further):

It seems that there is some error that has to do with TypeAdapter, and that is caused by the from future import annotations on the service.py, data_models.py and file_writer.py. After removing them the intersect service ran correctly (intersect-sdk0.8.3, 0.9.0 still fails with the first error). Is there a reason why those are needed?

The __future__ annotations was for Python 3.8 and 3.9 support, but we don't have that anymore in 0.9.0 . A significant reason why I wanted to make the minimum Python version 3.10 was to avoid dealing with __future__ imports precisely because they can cause subtle bugs like this.

@marshallmcdonnell so I can fix this like so:

1. (REQUIRED) Fix the error message, it's incorrect and misleading
2. (ERROR) Also allow hyphens, because the namespacing of Capabilities don't matter and I don't intend on basing MINIO bucket names around them

Thoughts?

[marshallmcdonnell]

I think just this is all we need to do:
Fix the error message, it's incorrect and misleading

So just (1).

I would rather not do (2) so it is very concise what we support or the right way to do it.