Skip to content

Conversation

Half-Shot
Copy link
Contributor

Trying to solve #466

I'm not sure if this works, but would value a review to see how close I am.

Copy link
Member

@tulir tulir left a comment

Choose a reason for hiding this comment

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

Should work for preventing portal creation, but the errors are probably not displayed in any sensible way currently.

@Half-Shot
Copy link
Contributor Author

This works for portals, but we'd also like to limit for puppeted users. Looking at that now...

Copy link

@jaller94 jaller94 left a comment

Choose a reason for hiding this comment

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

DBPortal.count() for some reason does not exceed 1.

Comment on lines 58 to 63
rows = cls.db.execute(select([func.count()]))
try:
count, = next(rows)
return count
except StopIteration:
return 0

Choose a reason for hiding this comment

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

Baby's first Python commits (don't commit blindly):

Suggested change
rows = cls.db.execute(select([func.count()]))
try:
count, = next(rows)
return count
except StopIteration:
return 0
return cls.db.query(func.count('*')).scalar()

Please test this. I haven't actually run this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No good unfortunately,

AttributeError: 'Engine' object has no attribute 'query'

Copy link
Member

Choose a reason for hiding this comment

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

The table to count rows in probably needs to be specified somewhere 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh derp, I thought that was implied somewhere

yield from cls._select_all()

@classmethod
def get_by_mxid(cls, mxid: RoomID) -> Integer:
Copy link
Member

Choose a reason for hiding this comment

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

this seems to have been changed accidentally


@classmethod
def count(cls) -> int:
count = cls.db.execute(select([func.count('*')]).select_from(Table("portal", MetaData()))).scalar()
Copy link
Member

Choose a reason for hiding this comment

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

I think this might work

Suggested change
count = cls.db.execute(select([func.count('*')]).select_from(Table("portal", MetaData()))).scalar()
count = cls.db.execute(cls.t.select([func.count('*')])).scalar()

Copy link

@jaller94 jaller94 left a comment

Choose a reason for hiding this comment

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

Tests showed that the room limit is enforced now. 👍

@Half-Shot
Copy link
Contributor Author

This needs to count the number of rooms where mxid is not null

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants