Add YAMLToJSONCustom to accept custom unmarshal func #65
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We want to use YAMLToJSON with yaml.v3 and this is the most minimal change that allows us to use yaml.v3 without breaking the interface of the rest of the library. Why v3? Because v3 changed how it treats boolean values: > YAML 1.1 bools (yes/no, on/off) are supported as long as they are > being decoded into a typed bool value. Otherwise they behave as a > string. Booleans in YAML 1.2 are true/false only. (https://github.com/go-yaml/yaml/blob/v3/README.md#compatibility)
mrnugget
added a commit
to sourcegraph/sourcegraph-public-snapshot
that referenced
this pull request
Jul 14, 2020
As we noticed last week: we can't use `on` as a key in a `CampaignSpec` because `yaml.YAMLToJSON` would convert that `on` into a `true` and that in turn would fail when validated. The solution here is to use a custom fork of `ghodss/yaml` that allows passing in a custom unmarshal-function which does _not_ do the conversion. The function we're passing in comes from the yaml.v3 library which changed its behavior when parsing boolean values (see ghodss/yaml#65). We lose the ability to use `YAMLToJSONStrict` since that only works with yaml.v2, but yaml.v3 already warns about duplicated keys and the JSON schema validation gives us enough of a safety net for the rest.
mrnugget
added a commit
to sourcegraph/sourcegraph-public-snapshot
that referenced
this pull request
Jul 14, 2020
As we noticed last week: we can't use `on` as a key in a `CampaignSpec` because `yaml.YAMLToJSON` would convert that `on` into a `true` and that in turn would fail when validated. The solution here is to use a custom fork of `ghodss/yaml` that allows passing in a custom unmarshal-function which does _not_ do the conversion. The function we're passing in comes from the yaml.v3 library which changed its behavior when parsing boolean values (see ghodss/yaml#65). We lose the ability to use `YAMLToJSONStrict` since that only works with yaml.v2, but yaml.v3 already warns about duplicated keys and the JSON schema validation gives us enough of a safety net for the rest.
eseliger
pushed a commit
to sourcegraph/sourcegraph-public-snapshot
that referenced
this pull request
Jul 15, 2020
As we noticed last week: we can't use `on` as a key in a `CampaignSpec` because `yaml.YAMLToJSON` would convert that `on` into a `true` and that in turn would fail when validated. The solution here is to use a custom fork of `ghodss/yaml` that allows passing in a custom unmarshal-function which does _not_ do the conversion. The function we're passing in comes from the yaml.v3 library which changed its behavior when parsing boolean values (see ghodss/yaml#65). We lose the ability to use `YAMLToJSONStrict` since that only works with yaml.v2, but yaml.v3 already warns about duplicated keys and the JSON schema validation gives us enough of a safety net for the rest.
eseliger
pushed a commit
to sourcegraph/sourcegraph-public-snapshot
that referenced
this pull request
Jul 16, 2020
As we noticed last week: we can't use `on` as a key in a `CampaignSpec` because `yaml.YAMLToJSON` would convert that `on` into a `true` and that in turn would fail when validated. The solution here is to use a custom fork of `ghodss/yaml` that allows passing in a custom unmarshal-function which does _not_ do the conversion. The function we're passing in comes from the yaml.v3 library which changed its behavior when parsing boolean values (see ghodss/yaml#65). We lose the ability to use `YAMLToJSONStrict` since that only works with yaml.v2, but yaml.v3 already warns about duplicated keys and the JSON schema validation gives us enough of a safety net for the rest.
mrnugget
added a commit
to sourcegraph/sourcegraph-public-snapshot
that referenced
this pull request
Jul 20, 2020
As we noticed last week: we can't use `on` as a key in a `CampaignSpec` because `yaml.YAMLToJSON` would convert that `on` into a `true` and that in turn would fail when validated. The solution here is to use a custom fork of `ghodss/yaml` that allows passing in a custom unmarshal-function which does _not_ do the conversion. The function we're passing in comes from the yaml.v3 library which changed its behavior when parsing boolean values (see ghodss/yaml#65). We lose the ability to use `YAMLToJSONStrict` since that only works with yaml.v2, but yaml.v3 already warns about duplicated keys and the JSON schema validation gives us enough of a safety net for the rest.
mrnugget
added a commit
to sourcegraph/sourcegraph-public-snapshot
that referenced
this pull request
Jul 22, 2020
As we noticed last week: we can't use `on` as a key in a `CampaignSpec` because `yaml.YAMLToJSON` would convert that `on` into a `true` and that in turn would fail when validated. The solution here is to use a custom fork of `ghodss/yaml` that allows passing in a custom unmarshal-function which does _not_ do the conversion. The function we're passing in comes from the yaml.v3 library which changed its behavior when parsing boolean values (see ghodss/yaml#65). We lose the ability to use `YAMLToJSONStrict` since that only works with yaml.v2, but yaml.v3 already warns about duplicated keys and the JSON schema validation gives us enough of a safety net for the rest.
mrnugget
added a commit
to sourcegraph/sourcegraph-public-snapshot
that referenced
this pull request
Jul 22, 2020
* WIP: New campaigns workflow * Remove old UI for switch to new campaigns workflow (#11891) * Add a basic implementation of the applyCampaign mutation (#11934) * update campaigns docs to reflect new flow (#11972) * update GraphQL API to remove unneeded namespace param, add applyCampaign test (#11982) * add applyCampaign resolver test, temporarily un-implement createCampaign The createCampaign mutation is strictly less powerful than applyCampaign, and it adds needless complexity to implement createCampaign now, especially since we'll be changing the impl of applyCampaign a lot. So, let's remove the createCampaign impl for now. Then we also need to change the resolver test from testing createCampaign to testing applyCampaign. * remove unneeded namespace param from createCampaign and applyCampaign mutations The namespace is immutably stored in the campaign spec, which is also provided as a param. * Implement moveCampaign mutation (#12049) * Fix campaigns web code after rebase * Implement ChangesetSpec resolver and validation of specs (#12092) * Implement ChangesetSpec.Description in resolver and data layer * Add ChangesetSpec/CampaignSpec schema and Validate() method * Validate ChangesetSpec against schema on create * Validate CampaignSpec against schema * Allow YAML as CampaignSpec input (caveat: 'on' needs to be quoted) * Fix critical whitespace error * Fix CampaignSpec.ParsedInput response * Remove debug log statement * Remove TODO comment * Fix indentation in raw specs for testing * Run prettier Co-authored-by: Erik Seliger <erikseliger@me.com> * Use fork of ghodss/yaml to use 'on' as key in CampaignSpec (#12153) As we noticed last week: we can't use `on` as a key in a `CampaignSpec` because `yaml.YAMLToJSON` would convert that `on` into a `true` and that in turn would fail when validated. The solution here is to use a custom fork of `ghodss/yaml` that allows passing in a custom unmarshal-function which does _not_ do the conversion. The function we're passing in comes from the yaml.v3 library which changed its behavior when parsing boolean values (see ghodss/yaml#65). We lose the ability to use `YAMLToJSONStrict` since that only works with yaml.v2, but yaml.v3 already warns about duplicated keys and the JSON schema validation gives us enough of a safety net for the rest. * Remove duplication in ChangesetSpec/CampaignSpec.UnmarshalValidate (#12156) * Remove repositoryDiffs (#12205) * Remove openChangesets resolver (#12209) We don't need it for the new UI anymore, so reducing the headache of maintaining it. * Implement minimal resolvers to incorporate schema changes (#12215) * Use same changeset resolver for hidden and visible changesets (#12221) * Fix compilation after rebase * Delete expired CampaignSpecs and ChangesetSpecs (#12210) * Fix compilation after rebase * Delete expired Campaign/ChangesetSpecs * Return correct ExpiresAt in Campaign/ChangesetSpec resolvers * Clean up tests for spec expiry * Remove unneeded Truncate Co-authored-by: Erik Seliger <erikseliger@me.com> * Remove unused dependency (#12245) * Implement placeholders for status and preview API (#12226) * Remove store.GetRepoIDsForFailedChangesetJobs (#12323) This was a left-over from the last PR that removed some functionality. I know that there's a lot more code that's technically unused now, but this one stood out since it had the really specific usecase of loading error messages for a campaign. * Another set of small follow-ups after introducing ExternalChangesetState (#12330) * Add a ChangesetState.Valid() method * Rename test method to reflect name of method under test * Update naming and un-export methods * Implement missing bits in CampaignSpec/ChangesetSpec resolvers (#12247) * Fix intendation of query in test * Implement CampaignSpec.ViewerCanAdminister * Add safety-check to ChangesetSpecDescription.Diff method * Clean up repository permissions test * Implement rest of ChangesetSpec and CampaignSpec resolvers * Add options to CountChangesetSpecsOpts * Implement changesetSpecConnectionResolver * Reduce test data after reducing test cases * Change type checks on ChangesetSpecDescription * Query __typename in test to make sure we get the right response * Use same repo query pattern in ChangesetResolver as in changesetSpecResolver * Implement the after param for changesetSpecs connection * Check for permissions in applyCampaign and moveCampaign * Add a test for ChangesetSpec.Description.Diff * Remove isHidden filter * Do not yield ChangesetSpec if repo is (soft-)deleted * Fix rebase gone wrong * Check namespace perms in CreateCampaignSpec/MoveCampaign (#12362) (This is part #11675 and the new workflow) Before this, every user could create a campaign spec in any org or user namespace. And the same was true for moveCampaign: a user could move a campaign into namespace they wanted. This changes both implementations in the service layer to check for the permissions of the current user (saved in the ctx): 1. If it's a site admin, they can create/move things in any namespace. 2. If not and the target namespace is an org, we check for org membership. 3. If not and the target namespace is a user, we check that it's the same as the current user. * Make schema for env more strict to only accept strings (#12335) Environment variables can only be strings, so we shouldn't accept int, bool etc. * Drop old campaigns tables and add missing columns (#12388) * Drop old campaigns tables and add missing columns This drops the following tables: - changeset_jobs - patches - patch_sets Why drop them? Since we decided we're going to migrate existing campaigns into read-only versions and that we don't want to keep working state (changeset_jobs) around, it doesn't make sense to keep the tables. Why keep empty tables? So, this removes the tables and all the code that depends on them. It also does two other things: - Add a `changeset_spec_id` column to `changesets` (that's not yet read/written to in the `campaigns.Store`) - Add three `diff_stat_*` columns to `changeset_specs`, populate them when creating a new `ChangesetSpec` and reading/writing them in the `campaigns.Store`. Why? Because I don't want to write another migration that adds diff stats to changeset specs in case those were created between the introduction of changeset specs and us adding the fields. * Do not show changeset count in frontend * Add notice to WIP campaigns docs that they're WIP (#12393) * Campaign frontend cleanup work (#12256) Co-authored-by: Quinn Slack <sqs@sourcegraph.com> Co-authored-by: Erik Seliger <erikseliger@me.com>
|
@ghodss ping? |
|
@bradfitz sorry for the ping, but it seems like you have recent activity in this repo. |
justinsb
pushed a commit
to justinsb/yaml
that referenced
this pull request
Jan 31, 2023
Non functional improvements
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We want to use YAMLToJSON with yaml.v3 and this is the most minimal
change that allows us to use yaml.v3 without breaking the interface of
the rest of the library.
Why v3? Because v3 changed how it treats boolean values: