Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Records.config ds profile parameters#3279

Merged
dangogh merged 3 commits into
apache:masterfrom
ezelkow1:records_config_ds_profile
Jan 31, 2019
Merged

Records.config ds profile parameters#3279
dangogh merged 3 commits into
apache:masterfrom
ezelkow1:records_config_ds_profile

Conversation

@ezelkow1
Copy link
Copy Markdown
Member

@ezelkow1 ezelkow1 commented Jan 29, 2019

What does this PR do?

Fixes #(issue_number)

Which TC components are affected by this PR?

  • Documentation
  • Grove
  • Traffic Analytics
  • Traffic Monitor
  • Traffic Ops
  • Traffic Ops ORT
  • Traffic Portal
  • Traffic Router
  • Traffic Stats
  • Traffic Vault
  • Other _________

What is the best way to verify this PR?

Add records.config parameters to a DS profile, make sure existing remaps stay the same

Check all that apply

  • This PR includes tests
  • This PR includes documentation updates
  • This PR includes an update to CHANGELOG.md
  • This PR includes all required license headers
  • This PR includes a database migration (ensure that migration sequence is correct)
  • This PR fixes a serious security flaw. Read more: www.apache.org/security

These changes allow you to put records.config parameters in to a DS profile and have them appear as remap overrides.  This is similar to how cachekey works in the DS profile. If the file given is for records.config then
options are generated for the remaps from the DS in this manner:
@plugin=conf_remap.so @pparam=param_name=value

This way you can do parameter overrides without having to use raw remap.

The parameters are expected to match the existing usages of records.config parameters in the scheme of:
CONFIG paramater_name INT/STRING/FLOAT value

outside of this the parameter will be rejected
…ces in case for some strange reason thats a thing
@asfgit
Copy link
Copy Markdown
Contributor

asfgit commented Jan 29, 2019

Can one of the admins verify this patch?

@mitchell852 mitchell852 added the Traffic Ops related to Traffic Ops label Jan 30, 2019
Comment thread traffic_ops/app/lib/API/Configs/ApacheTrafficServer.pm Outdated
Comment thread traffic_ops/app/lib/API/Configs/ApacheTrafficServer.pm Outdated
}

#Add DS profile records.config entries as overrided ATS parameters
if ( defined( $remap->{'param'}->{'records.config'} ) ) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this repeats the above -- would it make sense to make it a sub?

Copy link
Copy Markdown
Member Author

@ezelkow1 ezelkow1 Jan 30, 2019

Choose a reason for hiding this comment

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

Between the build_remap and the mid_remap there is a lot of similar duplication. It might make sense to make it a sub but it also follows the existing code convention thats there to just leave it as is

We may also want to treat edge/mid differently in the future depending on how this gets used (though will probably completely change if/when DS profiles get removed or all the ATS config gets moved in to golang) if we want a way to differentiate parameters that may only apply to one

@dangogh dangogh self-assigned this Jan 31, 2019
@dangogh dangogh merged commit dd32ff0 into apache:master Jan 31, 2019
@ezelkow1 ezelkow1 deleted the records_config_ds_profile branch January 31, 2019 17:44
@mitchell852 mitchell852 added new feature A new feature, capability or behavior cache-config Cache config generation labels May 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cache-config Cache config generation new feature A new feature, capability or behavior Traffic Ops related to Traffic Ops

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants