Properly escape colon within YAML

[jaysonvirissimo]

Hopefully closes this issue

[mverzilli]

@rousik this might point to a possible enhancement to the import script, right?

[emersonthis]

@jaysonvirissimo Thanks for this! It almost works but there's one more stray colon in another file. The second one appears to be a duplicate of the one you fixed, so it can maybe be deleted, but I don't know much about that section of content... Good work!

[jaysonvirissimo]

@emersonthis: I fixed the other file, but we should open a new issue for the duplicate resources. 👍

[emersonthis]

No more errors! And faster, too

[rousik]

We also need to fix _scripts/import_resources.py otherwise it will overwrite these changes on the next resource PR. What we probably need is 1. flag to allow for skipping over resources that already exist, 2. escaping of strings in the YML portion of the document.

[emersonthis]

@rousik Good catch. I don't know anything about the py script. Is this difficult to do? Are you able to coordinate with @jaysonvirissimo to make this happen? For the escaping issue, could it be as easy as just " quoting " every title? My understanding is that's still valid YAML, even if it's unnecessary in most cases. Just an idea.

[rousik]

I got https://github.com/flattenthecurve/guide/pull/305 to address these underlying problems.

[rousik]

I fixed the script now. I did not realize this snippet was yaml so now I'm using yaml.dump() to generate it and that takes care of the escaping properly. Thanks for highlighting this issue!

On Sun, Apr 5, 2020 at 12:00 PM emersonthis notifications@github.com wrote:

@rousik https://github.com/rousik Good catch. I don't know anything about the py script. Is this difficult to do? Are you able to coordinate with @jaysonvirissimo https://github.com/jaysonvirissimo to make this happen? For the escaping issue, could it be as easy as just " quoting " every title? My understanding is that's still valid YAML, even if it's unnecessary in most cases. Just an idea.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/flattenthecurve/guide/pull/301#issuecomment-609457106, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABYSCGFR5W4VYFGLIF6N6LLRLDBLBANCNFSM4L7YXNUQ .

[rousik]

I also got another PR https://github.com/flattenthecurve/guide/pull/306 which regenerates all existing resources and correctly escapes everything. That PR might be all we need instead of this localized fix.

On Sun, Apr 5, 2020 at 12:36 PM Jan Rouš rousik@gmail.com wrote:

I fixed the script now. I did not realize this snippet was yaml so now I'm using yaml.dump() to generate it and that takes care of the escaping properly. Thanks for highlighting this issue!

On Sun, Apr 5, 2020 at 12:00 PM emersonthis notifications@github.com wrote:

@rousik https://github.com/rousik Good catch. I don't know anything about the py script. Is this difficult to do? Are you able to coordinate with @jaysonvirissimo https://github.com/jaysonvirissimo to make this happen? For the escaping issue, could it be as easy as just " quoting " every title? My understanding is that's still valid YAML, even if it's unnecessary in most cases. Just an idea.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/flattenthecurve/guide/pull/301#issuecomment-609457106, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABYSCGFR5W4VYFGLIF6N6LLRLDBLBANCNFSM4L7YXNUQ .

[jaysonvirissimo]

I close this PR if/when this and this get merged. 👍

[jaysonvirissimo]

Closing, since this apparently fixed the root cause.