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

[v6r18] PilotsLogging DB/Service/Client #2918

Closed
wants to merge 64 commits into from

Conversation

miloszz
Copy link
Contributor

@miloszz miloszz commented May 12, 2016

DB, Service and Client for handling extended pilot logging.

Uses SQLAlchemy for database handling, DErrno for errors.

PR recreated for cleanup.

miloszz and others added 30 commits May 3, 2016 11:42
- adding tables for PilotsLogging using SQLAlchemy
- adding functions to handle DB operations for PilotsLogging
- added deleting from PilotsLogging when deleting pilot
- adding tables for PilotsLogging using SQLAlchemy
- adding functions to handle DB operations for PilotsLogging
- added deleting from PilotsLogging when deleting pilot
…Logging2

* 'PilotsLogging2' of github.com:miloszz/DIRAC: (27 commits)
  Fixed indentation
  Function auth moved to ConfigTemplate.cfg + typo spotted
  Adjusting filename for convention
  problem with case sensitiveness in filename
  Error handling using DErrno
  Code cosmetics
  Removing "from types import (...)"
  Code cosmetics
  Tests moved from TestDIRAC
  Removing "from types import (...)"
  Include PilotsLogging service in ConfigTemplate.cfg
  PilotsLogging, cherry-picking...
  Allow deleting PilotsLogging for list of PilotIDs.
  Tightening authorizations for handler functions
  PilotsLoggingDB.sql file for dirac-install integration.
  Dropping Core.Base.DB dependency - database is handled using SQLAlchemy, not DIRAC DB mechanism
  Code and comments cosmetics
  Deleting PilotsLogging when deleting pilot - updated + error handling
  Refactorisation of Pilots Logging. Database, service and client separated from PilotAgents
  Fixes in error handling
  ...
@landscape-bot
Copy link

Code Health
Repository health decreased by 0.07% when pulling 229f72d on miloszz:PilotsLogging2 into b509992 on DIRACGrid:rel-v6r15.

@fstagni
Copy link
Contributor

fstagni commented May 20, 2016

Content looks good to me. Please follow the advices given in landscape, especially see if the "12 errors" reported are real errors or just things that pylint does not understand (in this last case, use a per-line #pylint: disable=xxxxxxx

Also, doc should be added in this same PR: administration docs and developer docs.

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.04% when pulling 0cce84a on miloszz:PilotsLogging2 into b509992 on DIRACGrid:rel-v6r15.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 0cce84a on miloszz:PilotsLogging2 into * on DIRACGrid:rel-v6r15*.


Each logging entry includes:

- current status of the Pilot - has to be one of predefined list of possible states,
Copy link
Contributor

Choose a reason for hiding this comment

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

possible states: I understand this list of states is not in the current PR.
When such definition is available (in another PR) please link to it.

@fstagni
Copy link
Contributor

fstagni commented May 26, 2016

Great that also developer documentation has been added. It would be even greater if administrator documentation would be added, basically answering the following questions:

  • how to install this?
  • how to activate it?
  • how do I retrieve the pilots logging info?

@fstagni
Copy link
Contributor

fstagni commented May 26, 2016

The last point above made me to realize that there's no dirac script that, say, can answer the question "what are the pilots logging messages for pilot 12345?". Or am I missing something?

@fstagni
Copy link
Contributor

fstagni commented May 26, 2016

I suggest to add a line with

#pylint: disable=no-self-use

for handlers, as per construction these files will otherwise always show a "code smell" for every exposed function.


from sqlalchemy.sql.schema import ForeignKey, PrimaryKeyConstraint
from sqlalchemy.sql.sqltypes import DateTime
from sqlalchemy.orm import sessionmaker, scoped_session, mapper
Copy link
Contributor

Choose a reason for hiding this comment

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

mapper is unused

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling cb62e3a on miloszz:PilotsLogging2 into * on DIRACGrid:rel-v6r15*.

@landscape-bot
Copy link

Code Health
Repository health increased by 0.01% when pulling cb62e3a on miloszz:PilotsLogging2 into b509992 on DIRACGrid:rel-v6r15.

@miloszz
Copy link
Contributor Author

miloszz commented May 26, 2016

Still having some pylint errors and I'm afraid only way to avoid them is to add exception (or redesign DIRAC Service machinery). I think it will be for almost if not all services: in code global variables for databases are defined as Falses, when executing, real objects are assigned when service is started but pylint doesn't recognise it (Instance of 'bool' has no XXX member).

Also using global is pointed as something bad. As well as functions with many statements, many returns and many branches.

@landscape-bot
Copy link

Code Health
Repository health increased by 0.03% when pulling 9668d92 on miloszz:PilotsLogging2 into b509992 on DIRACGrid:rel-v6r15.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 9668d92 on miloszz:PilotsLogging2 into * on DIRACGrid:rel-v6r15*.

@fstagni
Copy link
Contributor

fstagni commented Aug 19, 2016

This branch has now conflicts that would need to be resolved. And the target branch should become v6r16.

@atsareg atsareg changed the title [v6r16] PilotsLogging DB/Service/Client [v6r18] PilotsLogging DB/Service/Client Nov 6, 2016
@miloszz miloszz closed this Nov 18, 2016
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.

None yet

4 participants