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

fix: api batch yaml separator #325

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

smehboub
Copy link
Contributor

@smehboub smehboub commented Jan 27, 2023

Hello,

When i execute the slo-generator as api mode with a batch request with some files sample, by example this one : https://github.com/google/slo-generator/blob/master/samples/cloud_monitoring/slo_gae_app_latency.yaml

$ slo-generator api -c config.yaml
$ curl -X POST -H "Content-Type: text/x-yaml" --data-binary @slo_gae_app_latency.yaml "http://localhost:8080?batch=true"

Because the yaml file separator is ";" in the batch process request method, you can see that here : https://github.com/google/slo-generator/blob/master/slo_generator/api/main.py#L205
And than the slo_gae_app_latency.yaml file has comments containing one ";", the file will be split to two parts to be compute
And we get the error (ERROR - SLO configuration is empty) in the output :

INFO - Loading slo-generator config from config.yaml
INFO - Loading config from HTTP request
INFO - Batch request detected. Splitting body and sending individual requests separately.
INFO - Sending # Copyright 2019 Google Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License") to HTTP batch handler.
INFO - Loading slo-generator config from config.yaml
INFO - Loading config from HTTP request
ERROR - SLO configuration is empty
INFO - Sending 
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#            http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
---
apiVersion: sre.google.com/v2
kind: ServiceLevelObjective

It is more standard and safer to use the yaml document markers as separator : https://yaml.org/spec/1.2.2/#document-markers

Currently, we can't send multiple yaml documents in one file with a standard way.

Thanks in advance for feedbacks.
Rgs,

@smehboub
Copy link
Contributor Author

Hello @lvaylet

Any news please ?

Thanks in advance.
Rgs,

Sophian

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.

1 participant