Skip to content
This repository was archived by the owner on Dec 20, 2025. It is now read-only.

fix(docker): Fix subscription leak in DockerTriggerTemplate#6874

Merged
anotherchrisberry merged 1 commit intospinnaker:masterfrom
dragon3:fix-subscription-leak-in-docker-trigger-template
Apr 23, 2019
Merged

fix(docker): Fix subscription leak in DockerTriggerTemplate#6874
anotherchrisberry merged 1 commit intospinnaker:masterfrom
dragon3:fix-subscription-leak-in-docker-trigger-template

Conversation

@dragon3
Copy link
Contributor

@dragon3 dragon3 commented Apr 23, 2019

WHAT

This pull request changes initialize function and added componentWillUnmount function to make sure the subscription unsubscribed properly.

WHY

I found an issue that Manual Execution modal sent duplicate HTTP request to "/images/tags" endpoint.

For example, if you click Start Manual Execution button, select a pipeline, 1 OPTION method and 1 GET method request, it's fine:

1

2

3


But if you select another pipeline, 2 same GET requests will be sent... :

4

And if you repeat select pipelines, the number of duplicate HTTP requests will be increased...

5


It seems that is caused by subscription leak in DockerTriggerTemplate.
This pull request is for fixing it.

This subscription leak causes duplicate requests to "/images/tags" endpoint.
@anotherchrisberry
Copy link
Contributor

Nice catch, thanks for the fix!

@anotherchrisberry anotherchrisberry merged commit 33309a4 into spinnaker:master Apr 23, 2019
anotherchrisberry added a commit that referenced this pull request Apr 23, 2019
* chore(core): Bump version to 0.0.354

96dcf58 refactor(stages): FormikStageConfig to provide Formik for StageConfigs (#6871)
bd94593 feat(kubernetes): remove rollout strategies feature flag
2056b64 fix(kubernetes): handle k8s-specific account/region task keys in tasks history view (#6869)

* chore(docker): Bump version to 0.0.37

33309a4 fix(docker): Fix subscription leak in DockerTriggerTemplate (#6874)
@dragon3 dragon3 deleted the fix-subscription-leak-in-docker-trigger-template branch April 24, 2019 00:30
@dragon3
Copy link
Contributor Author

dragon3 commented Apr 24, 2019

Thank you for the quick review and merge!

One question, are you going to merge (backport) this change into the previous versions? (1.13 and 1.12)

@anotherchrisberry
Copy link
Contributor

I'll defer to @maggieneterval on backporting this...

@dragon3
Copy link
Contributor Author

dragon3 commented Apr 24, 2019

Cool 👍
Thanks!

@maggieneterval
Copy link
Contributor

Hey @dragon3 good catch, we can definitely patch this into 1.12 and 1.13!

@maggieneterval
Copy link
Contributor

@spinnakerbot cherry-pick 1.12

@maggieneterval
Copy link
Contributor

@spinnakerbot cherry-pick 1.13

spinnakerbot pushed a commit that referenced this pull request Apr 24, 2019
This subscription leak causes duplicate requests to "/images/tags" endpoint.
@spinnakerbot
Copy link
Contributor

Cherry pick successful: #6887

spinnakerbot pushed a commit that referenced this pull request Apr 24, 2019
This subscription leak causes duplicate requests to "/images/tags" endpoint.
@spinnakerbot
Copy link
Contributor

Cherry pick successful: #6888

maggieneterval pushed a commit that referenced this pull request Apr 24, 2019
This subscription leak causes duplicate requests to "/images/tags" endpoint.
maggieneterval pushed a commit that referenced this pull request Apr 24, 2019
This subscription leak causes duplicate requests to "/images/tags" endpoint.
@dragon3
Copy link
Contributor Author

dragon3 commented Apr 25, 2019

Nice 👍
Thank you for patching into 1.12 and 1.13!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants