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

feat(pyproject): add N (pep8 naming) rule to ruff config... #334

Merged
merged 3 commits into from
Dec 20, 2024

Conversation

aldbr
Copy link
Contributor

@aldbr aldbr commented Dec 2, 2024

...Before it becomes too inconsistent.

Partially solves #34

It seems there is no clear choice for the attributes of the SQL Alchemy classes and we actually write them in 2 different ways:

  • snake case (this is how it is presented in the SQLAlchemy documentation):

    class DeviceFlows(Base):
    __tablename__ = "DeviceFlows"
    user_code = Column(String(USER_CODE_LENGTH), primary_key=True)
    status = EnumColumn(FlowStatus, server_default=FlowStatus.PENDING.name)
    creation_time = DateNowColumn()
    client_id = Column(String(255))
    scope = Column(String(1024))
    device_code = Column(String(128), unique=True) # Should be a hash
    id_token = NullColumn(JSON())

  • camel case:

    class JobJDLs(JobDBBase):
    __tablename__ = "JobJDLs"
    JobID = Column(Integer, autoincrement=True, primary_key=True)
    JDL = Column(Text)
    JobRequirements = Column(Text)
    OriginalJDL = Column(Text)

We should probably make an opinionated choice and specify it in CODING_CONVENTION.md.
Any opinion?

Needs: DIRACGrid/DIRAC#7968

@aldbr aldbr force-pushed the main_FIX_camel-case-job-db branch 4 times, most recently from 8f0add5 to f0a7ef4 Compare December 2, 2024 08:51
@fstagni
Copy link
Contributor

fstagni commented Dec 2, 2024

snake_case ...?

@aldbr aldbr force-pushed the main_FIX_camel-case-job-db branch 2 times, most recently from a91be64 to f006bb0 Compare December 2, 2024 10:01
@chaen
Copy link
Contributor

chaen commented Dec 2, 2024

snake_case definitely, BUT the reason for having the Camel case is the compatibility with DIRAC :-) it just started because of the prototype.
So indeed, the attributes should be xnake case, but the column name has to be Caml case

@aldbr aldbr force-pushed the main_FIX_camel-case-job-db branch 2 times, most recently from a7524c8 to bc4ac8a Compare December 3, 2024 07:59
@aldbr
Copy link
Contributor Author

aldbr commented Dec 3, 2024

Alright, I converted the attributes to snake_case and the column name in PascalCase.
So even the Auth SQLAlchemy classes, which are new, follow the same pattern in order to remain consistent.

@aldbr
Copy link
Contributor Author

aldbr commented Dec 3, 2024

I guess this error is expected: https://github.com/DIRACGrid/diracx/actions/runs/12135284425/job/33834028304?pr=334#step:14:205

Because I modified the DB schema of AuthDB. The pipeline should be fixed once the PR will be merged if I understand correctly (the new schema will be included in the diracx/services:dev image).

@chaen chaen force-pushed the main_FIX_camel-case-job-db branch from bc4ac8a to b17c67d Compare December 3, 2024 10:40
@aldbr aldbr force-pushed the main_FIX_camel-case-job-db branch 2 times, most recently from 0d8b225 to 782cfee Compare December 3, 2024 13:03
@aldbr aldbr mentioned this pull request Dec 13, 2024
6 tasks
@chrisburr chrisburr force-pushed the main_FIX_camel-case-job-db branch 2 times, most recently from f794af9 to c9e848d Compare December 17, 2024 14:36
@aldbr aldbr force-pushed the main_FIX_camel-case-job-db branch 4 times, most recently from a3037f2 to 34d1398 Compare December 20, 2024 08:37
@aldbr aldbr reopened this Dec 20, 2024
@chrisburr chrisburr merged commit 320c554 into DIRACGrid:main Dec 20, 2024
46 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants