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

Simplify logic for prefixing fastly spec to file #345

Merged
merged 2 commits into from Jul 15, 2021

Conversation

Integralist
Copy link
Collaborator

Some users weren't seeing the fastly.toml spec URL in their configuration file (the logic was working for the majority of users who tested the code, but there appears to be an intermittent issue which would cause the spec URL insertion to fail).

An internal recommendation was made to simplify the logic (described below), and that's what has been done in this PR (which we expect to also resolve the issue reported internally with the spec URL being omitted).

The original flow was to encode data to disk and then seek the start of the file, checking if the spec URL was absent, and if so prepend it to the file. This complex flow was done because previously the toml dependency would truncate the entire file and remove the fastly spec URL, but since changing to a new (maintained) toml dependency that truncation no longer happens when encoding data to disk.

This means the flow for writing the config back to disk is now simply:

  • Truncate fastly.toml.
  • Write spec URL to disk.
  • Encode the configuration data to disk (leaving the existing content unaffected).

@Integralist Integralist added the enhancement New feature or request label Jul 15, 2021
@Integralist Integralist requested review from a team, triblondon and kailan and removed request for a team July 15, 2021 10:05
@Integralist Integralist merged commit af8d5db into main Jul 15, 2021
@Integralist Integralist deleted the integralist/simplify-prepend-fastly-spec branch July 15, 2021 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants