feat: Check for __all__ presence in __init__.py files#766
feat: Check for __all__ presence in __init__.py files#766AcquaDiGiorgio wants to merge 10 commits intoDIRACGrid:mainfrom
Conversation
fix: Exclude automatically generated __init__ files
I didn't see any standard way of defining |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
chrisburr
left a comment
chrisburr
left a comment
There was a problem hiding this comment.
Yet another fake review for me to be able to test something 😄
|
I don't know how much of an improvement this is, but we might not want to use importlib for this task. To import an spec = importlib.util.spec_from_file_location(module_name, file)
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)Here, While importing the module, if the However, as a counterpart of this method, importing the module each time is quite slow, and I don't see any issues on using regex. Also, I found a wild |
|
Note: I keep the PR as a draft to not interrupt whatever test you are doing, but is ready to review. |
No, I don't think so. Can you remove it please? |
|
This every dependency I have extracted from the imports on every file: Sometimes a module is imported, such as I tried to put in the from __future__ import annotations
__all__ = [
"SecurityProperty",
"UnevaluatedProperty",
"PROXY_MANAGEMENT",
"GENERIC_PILOT",
"JOB_ADMINISTRATOR",
"NORMAL_USER",
"JOB_SHARING",
"s3_bucket_exists",
"generate_presigned_upload",
"s3_bulk_delete_with_retry",
"s3_object_exists",
"find_compatible_platforms",
"ServiceSettingsBase",
"AuthSettings",
"DevelopmentSettings",
"SandboxStoreSettings",
"SqlalchemyDsn",
"DiracEntryPoint",
"select_from_extension",
"supports_extending",
"DiracError",
"DiracHttpResponseError",
"NotReadyError",
"InvalidCredentialsError",
"TokenNotFoundError",
"AuthorizationError",
"IAMClientError",
"IAMServerError",
"PendingAuthorizationError",
"SandboxAlreadyAssignedError",
"SandboxNotFoundError",
"SandboxAlreadyInsertedError",
"InvalidQueryError",
"ConfigSource",
"Config",
"ConfigSourceUrl",
"DiracxPreferences",
"get_diracx_preferences",
"OutputFormats",
"EXPIRES_GRACE_SECONDS",
"serialize_credentials",
"EXPIRES_GRACE_SECONDS",
"serialize_credentials",
"dotenv_files_from_environment",
"read_credentials",
"write_credentials",
]
from .config import Config, ConfigSource, ConfigSourceUrl
from .exceptions import (
AuthorizationError,
DiracError,
DiracHttpResponseError,
IAMClientError,
IAMServerError,
InvalidCredentialsError,
InvalidQueryError,
NotReadyError,
PendingAuthorizationError,
SandboxAlreadyAssignedError,
SandboxAlreadyInsertedError,
SandboxNotFoundError,
TokenNotFoundError,
)
from .extensions import (
DiracEntryPoint,
select_from_extension,
supports_extending,
)
from .preferences import (
DiracxPreferences,
OutputFormats,
get_diracx_preferences,
)
from .properties import (
GENERIC_PILOT,
JOB_ADMINISTRATOR,
JOB_SHARING,
NORMAL_USER,
PROXY_MANAGEMENT,
SecurityProperty,
UnevaluatedProperty,
)
from .resources import find_compatible_platforms
from .s3 import (
generate_presigned_upload,
s3_bucket_exists,
s3_bulk_delete_with_retry,
s3_object_exists,
)
from .settings import (
AuthSettings,
DevelopmentSettings,
SandboxStoreSettings,
ServiceSettingsBase,
SqlalchemyDsn,
)
from .utils import (
EXPIRES_GRACE_SECONDS,
dotenv_files_from_environment,
read_credentials,
serialize_credentials,
write_credentials,
)Is this what we want? |
|
Here is an example of what we want I think:
I didn't check all the sub directories and modules, it's just to give you a sense of what we should probably aim for. |
|
Yeah, that makes sense, the example of For example, the diracx-routers project: You said that in this case If so, those dependencies should be in the the |
Some of them could be if there are meant to be used in the extensions indeed. You can have a look at the If you have any doubt, you can present the different options with pros and cons 🙂 |
|
Okay, after looking through lhcbdiracx a bit , then maybe the best way of proceeding would be:
More or less the same way as you said, but taking into account the modules at root I could change the pre-commit hook to check modules at the root if we go with this |
Things to check out
|
|
This ended up being a huge change, we should discuss it further |
3 participants
See #426
I tried something more sofisticated in python using
importlibto check the variables in the module, both instantiated and imported, but there were multiple situations that were extremely difficult to check.We also might want to set a common way of defining this variable, using either tuples or lists:
__all__ = ["router"]__all__ = ("JobParametersDB",)