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

Update the fastly.toml manifest if missing manifest_version. #220

Merged
merged 2 commits into from Mar 17, 2021

Conversation

Integralist
Copy link
Collaborator

Problem: the error message (see below) was being shown every time fastly compute build was executed. This happened because although the logic set a correct manifest_version value, that only happened in-memory and so the same error would happen each time the CLI was used (unless the user manually updated the file).

The fastly.toml was missing a `manifest_version` field. A default schema version of `1` will be used.

Additionally to this I discovered that the current implementation wouldn't write the manifest_version to the file correctly. It would be written as a separate block [manifest_version] rather than as a top-level field. So I've refactored the logic to fix that bug.

@Integralist Integralist added the bug Something isn't working label Mar 17, 2021
@Integralist Integralist requested review from phamann, a team and kailan and removed request for a team March 17, 2021 16:02
Copy link
Member

@kailan kailan left a comment

Choose a reason for hiding this comment

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

Got a weird dynamic config related error but I don't think it's anything to do with these changes.

kailan@KB-Fastly not-real-cli-config % ../../fastly/cli/fastly compute build

WARNING: The fastly.toml was missing a `manifest_version` field. A default schema version of `1` will be used.

✓ Initializing...
✓ Verifying package manifest...
- Checking if Rust 1.49.0 is installed...
ERROR: Get "https://developer.fastly.com/api/internal/cli-config": context deadline exceeded.

This error may be caused by transient network issues. Please verify your network connection and try again.

kailan@KB-Fastly not-real-cli-config % ../../fastly/cli/fastly compute build
✓ Initializing...
✓ Verifying package manifest...
✓ Verifying local rust toolchain...
✓ Building package using rust toolchain...
✓ Creating package archive...

SUCCESS: Built rust package cli-config (pkg/cli-config.tar.gz)

The manifest_version functionality works as intended. Test case looks good. Ship it 🚀

@Integralist Integralist merged commit 1b41384 into master Mar 17, 2021
@Integralist Integralist deleted the integralist/20210317_write_after_read branch March 17, 2021 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants