add a user database #3
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "user-db"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This PR aims to add a user database to to the git importer and enable import and batch import of users from the OBS
@ -11,2 +11,4 @@
from lib.proxy_sha256 import ProxySHA256, md5, sha256
from lib.request import Request
import xml.etree.ElementTree as ET
from lib.users import User
Please rename that to lib.user
@ -159,2 +161,4 @@
self.update_db_package(db, lproject, lpackage)
missing_users = User.get_batch(db)
print(missing_users)
print left
@ -161,0 +167,4 @@
if missing_user is not None:
missing_user.import_into_db(db)
else:
logging.info("No missing users")
That else is strange. If there are no missing users, I would expect get_batch returns an empty list and you don't even end up in the for loop's body
Indeed.
IMHO avoid
is None
,== []
. Use the truth value (if missing_user:
)@ -173,6 +184,10 @@ class Importer:
else:
rev.set_broken(db)
# fake_accounts = ["unknown","buildservice-autocommit", "autobuild", "_service"]
Dead code?
@ -0,0 +5,4 @@
self.userid = userid
self.realname = xml.find("realname").text
self.email = xml.find("email").text
if self.email == None:
is None
@ -0,0 +20,4 @@
cur = db.cursor()
cur.execute(
"""INSERT INTO users (userid, realname, email)
VALUES (%s,%s,%s) RETURNING id""",
if you don't need the id, don't have it RETURNING
@ -0,0 +29,4 @@
)
cur.close()
def lookup(db, userid):
this function is part of the dead code
I think there is value in keeping this for now to look up specific users
Then it needs to be a class method
@staticmethod
Classmethod or Staticmethod? I would have defaulted to classmethod but I can change it either way
@staticmethod
, those are method associated with a class and not an instance.@classmethod
are use for metaprogramming, when you want to change the class definition (weird stuff).In a different comment I linked a bit about this.
@ -0,0 +39,4 @@
return row
@classmethod
def get_batch(self, db):
Make that missing_users.
@ -0,0 +48,4 @@
('unknown','buildservice-autocommit', 'autobuild', '_service')"""
)
missing_users = [row[0] for row in cur.fetchall()]
if not missing_users:
always close the cursor and just return missing_users. It's always a list, possibly an empty one - which you can return when there are no missing_users
@ -176,0 +187,4 @@
# fake_accounts = ["unknown","buildservice-autocommit", "autobuild", "_service"]
# if User.user_lookup(db, rev.userid) is None and rev.userid not in fake_accounts:
# self.obs.users(rev.userid).import_into_db(db)
Remove this section?
@ -125,0 +134,4 @@
def user(self, userid):
root = self._user(userid)
if root is not None:
return User().parse(root, userid)
It is OK how you write it, but I would expect
root
anduserid
as part of the__init__
constructor (give initial state).root not as it's OBS specific while we want to have Users also coming from the DB
@ -0,0 +1,55 @@
FAKE_ACCOUNTS = ('unknown','buildservice-autocommit', 'autobuild', '_service')
For 4 elements a tuple is totally OK. If you expect that it will grow, use a
set
.Missing space between the first and second element (I recommend you to use
black
to normalize the style)I actually added a Makefile so
make all
runs all the formaters I use :)@ -0,0 +5,4 @@
self.userid = userid
self.realname = xml.find("realname").text
self.email = xml.find("email").text
if self.email == None:
is
,black
will detect this kind of issues too (I think)@ -0,0 +39,4 @@
return row
@classmethod
def get_batch(self, db):
If it is a class method do not expect
self
as a first parameter, it is usuallycls
[1]Besides, I think that you whant
@staticmethod
here[2], in that case dropself
[1] https://docs.python.org/3/library/functions.html#classmethod
[2] https://docs.python.org/3/library/functions.html#staticmethod
That's obviously true for other methods I added, so blame me :)
@ -0,0 +40,4 @@
@classmethod
def get_batch(self, db):
cur = db.cursor()
Use a
with
context, like in a file: https://www.psycopg.org/docs/cursor.htmlShould I refactor the code to use context managers everywhere?
As you wish.
that would be awesome!
If you feel strong, refactor also the
schema = dict()
part, and build a list : ]I guess we need to still define an email/username for fake accounts and unknown users. But this can be a 2nd step.
@ -175,0 +160,4 @@
(lproject, lpackage) = row
self.update_db_package(db, lproject, lpackage)
missing_users = User.missing_users(db)
I think that from here you can remove all from the
with
context (at least I do not see any access tocur
)