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

Expose DB name attribute at the Python level #90

Merged
merged 1 commit into from
Nov 14, 2018

Conversation

sbellem
Copy link
Contributor

@sbellem sbellem commented Oct 30, 2018

Related to issue #88.

The DB.name attribute is not exposed at the Python level and could sometimes be useful. One example, is the deletion of a DB instance for which the name has not been "cached" at creation time. One could get the name by parsing DB.__str__() or DB.__repr__() but why not directly from DB.name?

@wbolster
Copy link
Owner

thanks, is there a way to make the attribute read only?

@sbellem
Copy link
Contributor Author

sbellem commented Oct 30, 2018

thanks, is there a way to make the attribute read only?

Oh, that is a very good point. Will do!

@sbellem
Copy link
Contributor Author

sbellem commented Oct 30, 2018

@wbolster See re-worked commit. The DB.name attribute is now readonly at the Python level. There is a small test to check that an error is raised if one attempts to change the name.

Just in case that helps, I found the following docs useful https://cython.readthedocs.io/en/latest/src/userguide/extension_types.html#static-attributes.

Copy link
Owner

@wbolster wbolster left a comment

Choose a reason for hiding this comment

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

awesome, looking good.

can you add the documentation to the api docs with a proper version tag? (use 1.1.0)

@sbellem
Copy link
Contributor Author

sbellem commented Nov 4, 2018

can you add the documentation to the api docs with a proper version tag? (use 1.1.0)

Yes!

@sbellem
Copy link
Contributor Author

sbellem commented Nov 5, 2018

@wbolster Added docs with a note about the version. Let me know if anything else needs to be done.

Copy link
Owner

@wbolster wbolster left a comment

Choose a reason for hiding this comment

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

looks cool

@wbolster wbolster merged commit 1a04cfb into wbolster:master Nov 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants