- Notifications
You must be signed in to change notification settings - Fork673
introduce RefreshMixin#426
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
docs/gl_objects/projects.py Outdated
| @@ -333,6 +333,22 @@ | |||
| pipeline = project.pipelines.create({'ref': 'master'}) | |||
| # end pipeline create | |||
| # pipeline trigger | |||
| def get_or_create_trigger(project): | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think this could be a nice improvement to add this get_or_create to trigger manager
docs/gl_objects/projects.py Outdated
| trigger = get_or_create_trigger(project) | ||
| pipeline = project.trigger_pipeline('master', trigger.token, variables={"DEPLOY_ZONE": "us-west1"}) | ||
| while pipeline.finished_at is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
also, we could add a helper likepipeline.wait_for_finish(poll_freq=1)
@gpocentek could you please give some feedback? |
Hi@tardyp Sorry for my late answer. If I understand correctly the goal of the ReadMixin is to refresh the attributes an existing object. I like the idea but I wonder if adding a mixin is really the best option. What do you think of adding a Does that sound good to you? Thanks for your work :) |
@gpocentek I like the refresh name method better than read. Thanks for the suggestion. I think that the ReadMixin is only for managers. Here we want to add this mixin to restobjects. Let me know your though, I'll update the PR after we get an agreement. |
@tardyp you're right, using GetMixin doesn't make sense. I don't think it will be possible to refresh all the objects, so I guess providing a new RefreshMixin with a refresh method is the way to go. Thank you for the update. |
RefreshMixin allows to update a REST object so that you can poll on it.This is mostly useful for pipelines and jobs, but could be set on most of other objects, with unknown usecases.
Hi@gpocentek I updated my code. I believe the e2e tests are still a little bit flaky. |
Hi@tardyp The tests have been a problem for a little while now, but I didn't have time to try to understand why yet. Do you mind removing the change on the test script? I don't think it will really help, and I'd prefer to have this kind of change in a separate path. Apart from that the patch looks good, thanks! |
@gpocentek done |
Thanks! |
Uh oh!
There was an error while loading.Please reload this page.
RefreshMixin allows to update a REST object so that you can poll on it.
This is mostly useful for pipelines and jobs, but could be set on most of other objects, with unknown usecases.