Skip to content

Conversation

senior
Copy link
Contributor

@senior senior commented Sep 10, 2015

Prior to this commit, any non-ASCII character found in a catalog would
be replaced by /ufffd and a warning would be issued saying it found an
invalid character. The code now takes into account the force_encoding
done by to_pson and attempts to force_encode it back to UTF-8. For most
cases this should be sufficient.

There are times when binary data can appear in a catalog. In this case,
we have characters that we can't represent in UTF-8. For this we
continue to give users a warning. This warning now includes the command
being submitted and the node the command is for. If users have debug
logging enabled, context is given around where this bad character data
occurred. Up to 100 characters before/after this bad data is included in
the Puppet debug log.

@puppetlabs-jenkins
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://jenkins-enterprise.delivery.puppetlabs.net/job/enterprise_puppetdb_init-multijob_githubpr-master/46/
Test PASSed.

@kbarber kbarber added this to the 3.2.0 (master) milestone Sep 11, 2015
Copy link
Contributor

Choose a reason for hiding this comment

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

should I see this getting logged with this catalog?

exec {"\u006E\u0303":
  command => "/usr/bin/uptime"
}

exec {"\u00F1":
  command => "/usr/bin/date"
}

I see this:

Warning: Ignoring invalid UTF-8 byte sequences in data to be sent to PuppetDB

in the puppet log, but no mention of replace catalog specifically.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh sorry, not on your code

@wkalt
Copy link
Contributor

wkalt commented Sep 11, 2015

+1 from me. functionality looks good.

@senior senior force-pushed the ticket/master/pdb-135-better-utf8-warning branch from 2efcd61 to e109a6c Compare September 15, 2015 14:54
@senior
Copy link
Contributor Author

senior commented Sep 15, 2015

@ajroetker reworked collapse_ranges based on your feedback, should be good to go now

Prior to this commit, any non-ASCII character found in a catalog would
be replaced by /ufffd and a warning would be issued saying it found an
invalid character. The code now takes into account the force_encoding
done by to_pson and attempts to force_encode it back to UTF-8. For most
cases this should be sufficient.

There are times when binary data can appear in a catalog. In this case,
we have characters that we can't represent in UTF-8. For this we
continue to give users a warning. This warning now includes the command
being submitted and the node the command is for. If users have debug
logging enabled, context is given around where this bad character data
occurred. Up to 100 characters before/after this bad data is included in
the Puppet debug log.
@senior senior force-pushed the ticket/master/pdb-135-better-utf8-warning branch from e109a6c to d9ae1d7 Compare September 15, 2015 14:59
@ajroetker
Copy link
Contributor

Lgtm 👍

@puppetlabs-jenkins
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://jenkins-enterprise.delivery.puppetlabs.net/job/enterprise_puppetdb_init-multijob_githubpr-master/59/
Test FAILed.

@puppetlabs-jenkins
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://jenkins-enterprise.delivery.puppetlabs.net/job/enterprise_puppetdb_init-multijob_githubpr-master/60/
Test PASSed.

wkalt added a commit that referenced this pull request Sep 15, 2015
…8-warning

(PDB-135) Improve and reduce the number of UTF-8 warnings
@wkalt wkalt merged commit cc516ba into puppetlabs:master Sep 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants