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

fix(deck): Removal of API #10017

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

AbhaHimani
Copy link

Removal of loadBalancer API call from the website's loading page

@spinnakerbot
Copy link
Contributor

The following commits need their title changed:

Please format your commit title into the form:

<type>(<scope>): <subject>, e.g. fix(kubernetes): address NPE in status check

This allows us to easily generate changelogs & determine semantic version numbers when cutting releases. You can read more about commit conventions here.

@mattgogerly
Copy link
Member

Could you explain what you're trying to fix here?

@AbhaHimani
Copy link
Author

Yes, sure!
I analysed that when you open the executions URL page, the loadBalancer API call is being made, which is not used anywhere in the UI/functionality of the pipelines tab, so the loadBalancer API call can be removed, to remove it I have analysed the two parameters that can help to call the loadBalancer API only when it is needed, so, I have considered the activeState object as welll. Let me know if you need any further explanation! :)

@mattgogerly
Copy link
Member

Sorry, been slammed recently. Making a note to myself to come back to this - I want to double check the flow as I think the calls made are controlled elsewhere too.

@AbhaHimani
Copy link
Author

Sure sure! Let me know

@mattgogerly
Copy link
Member

Sorry again, finally found some time to look at this.

This method is responsible for the application "refresh" loop. It already has a filter on datasources that are not visible, and appears to refresh them anyway, but does not wait for them to complete before considering the refresh as complete.

I suspect this change would work better here - perhaps we want to change the logic of this method to just not refresh non-visible pages at all? That would apply this change to all tabs rather than just the load balancers one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants