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

ui: Remove writable usage from policy and use the request instead #6934

Merged
merged 1 commit into from
Dec 17, 2019

Conversation

johncowen
Copy link
Contributor

@johncowen johncowen commented Dec 11, 2019

A recent change to the backend (#6874) meant that if you sent any data in the HTTP body that wasn't needed the backend would return an error.

In #6917 we changed the HTTP requests that weren't already using our writable functionality to remove data from the HTTP request that wasn't needed.

writable was used before we rewrite our data to achieve this, but our new data layer makes it much easier to see what is being sent to the server as it doesn't spread the information for the HTTP request over multiple files.

This PR remove writable usage from policy and continues to clean up token and moves them to use our new data layer approach.

We can't quite delete writable yet as in this branch intentions still use it, but in #6639 we switched intentions to use our new approach also while we were there. Once #6639 and this PR are merged we can add another PR to completely remove writable 😁

@johncowen johncowen requested a review from a team December 11, 2019 14:32
@johncowen johncowen added the theme/ui Anything related to the UI label Dec 11, 2019
@codecov
Copy link

codecov bot commented Dec 11, 2019

Codecov Report

Merging #6934 into ui-staging will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##           ui-staging    #6934      +/-   ##
==============================================
- Coverage       65.66%   65.65%   -0.01%     
==============================================
  Files             443      443              
  Lines           53312    53292      -20     
==============================================
- Hits            35006    34990      -16     
+ Misses          14086    14085       -1     
+ Partials         4220     4217       -3
Impacted Files Coverage Δ
api/session.go 70.68% <0%> (-5.18%) ⬇️
agent/consul/operator_autopilot_endpoint.go 46% <0%> (-4%) ⬇️
agent/http_oss.go 50% <0%> (-2.64%) ⬇️
agent/consul/replication.go 85.71% <0%> (-2.39%) ⬇️
agent/consul/config_replication.go 75.51% <0%> (-2.05%) ⬇️
agent/consul/autopilot/autopilot.go 6.85% <0%> (-2.01%) ⬇️
command/connect/envoy/envoy.go 44.79% <0%> (-1.96%) ⬇️
agent/local/state.go 76.12% <0%> (-1.45%) ⬇️
api/semaphore.go 80.17% <0%> (-0.87%) ⬇️
agent/service_manager.go 81.62% <0%> (-0.86%) ⬇️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b330b80...ccc2f5b. Read the comment docs.

@johncowen johncowen force-pushed the feature/ui-almost-remove-writable branch from d60bbaa to 4316a44 Compare December 12, 2019 13:28
@johncowen johncowen force-pushed the feature/ui-almost-remove-writable branch from 4316a44 to ccc2f5b Compare December 17, 2019 15:58
Copy link
Contributor

@pearkes pearkes left a comment

Choose a reason for hiding this comment

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

Looks good, it feels like the new approach is easier to understand (without me knowing much about writeable).

@johncowen johncowen merged commit a89a006 into ui-staging Dec 17, 2019
@johncowen johncowen deleted the feature/ui-almost-remove-writable branch December 17, 2019 16:16
@ghost
Copy link

ghost commented Jan 25, 2020

Hey there,

This issue has been automatically locked because it is closed and there hasn't been any activity for at least 30 days.

If you are still experiencing problems, or still have questions, feel free to open a new one 👍.

@ghost ghost locked and limited conversation to collaborators Jan 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
theme/ui Anything related to the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants