-
Notifications
You must be signed in to change notification settings - Fork 670
methods deprecation #280
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
Comments
Hi @guyzmo :) Could you explain a little more why you need to instantiate the request session first? I'm trying to understand the use case. My idea with the change is that it doesn't really make sense to have a gitlab object without its authentication and URL parameters. But maybe I missed something... Thanks! |
the process is the following:
Fundamentally, the reason I need to have the session the earliest is because of [betamax(https://github.com/sigmavirus24/betamax) which hijacks the requests session, at construction time. So if you really want to go the way you're thinking about, then add a However you think about it, you're not supposed to make a request to the gitlab server before I explicitely issue a |
(added a link to the code where I use it in the issue) https://github.com/guyzmo/git-repo/blob/devel/git_repo/services/ext/gitlab.py#L34,L36 |
I see! I think you don't need the deprecated methods: https://github.com/gpocentek/git-repo/tree/python-gitlab/280 Let me know if this looks OK. |
cf my comments your PR won't pass the tests, as I won't have access to So it's either dependency injection (with the BTW, see how the same thing is done with sigmavirus24/github3.py. and the hack I had to do to improve the python-gogs lib for my needs, used there. |
So, how do you think shall be the way ahead? Dependency injection of the |
basically, have something like that, in the gitlab constructor: def __init__(self, url, private_token=None, email=None, password=None,
ssl_verify=True, http_username=None, http_password=None,
timeout=None, session=None, api_version='3'):
# …
#: Create a session object for requests
self.session = session or requests.Session()
# … |
fixes python-gitlab#280 Signed-off-by: Guyzmo <guyzmo+github+pub@m0g.net>
fixes python-gitlab#280 Signed-off-by: Guyzmo <guyzmo+github+pub@m0g.net>
hop @gpocentek here you go! Just pushed a 2 lines patch that will make me happy, and shouldn't change much of your new design. I'm preparing a PR on my side for using that patch 👍 |
(N.B.: any ETA for this change to make it to pypi? Just to sync my next release up with yours 😉) |
@guyzmo I'd like to merge my v4 branch first. It's a big change and I need a bit more time. I'll probably ping you to validate it BTW, you're the only downstream project maintainer I know ;) |
Sure!
cool 👍 BTW I'm also
That's because I'm the nicer one 😁 |
fixes python-gitlab#280 Signed-off-by: Guyzmo <guyzmo+github+pub@m0g.net>
I've had a report for deprecation warnings in git-repo when using gitlab. I then looked at the release notes, and found out that a bunch of the
Gitlab
object's API is getting deprecated.Though, I actually need those in git-repo where I initialise objects in two steps, first I initialise the instances (and importantly request's session) then I load the configuration, and finally the connect method configures the gitlab object.
So either having a method to configure the object after it has been instanciated (with an API similar to the constructor), or keeping the setters really matter to me! Or I'll end up forced to monkey patched internals of
python-gitlab
and that can end up ugly 😖cf this issue
Thank you for your lib ♥
The text was updated successfully, but these errors were encountered: