| Message ID | 20250408100945.111801-1-jaeyoon.jung@lge.com |
|---|---|
| State | New |
| Headers | show |
| Series | send-error-report: Allow to set server using an environment variable | expand |
On Tue, 8 Apr 2025 at 12:10, Jaeyoon Jung (LGE) via lists.openembedded.org <jaeyoon.jung=lge.com@lists.openembedded.org> wrote: > > From: Jaeyoon Jung <jaeyoon.jung@lge.com> > > Check ERROR_REPORT_SERVER if -s or --server isn't set before falling back > to the default errors.yoctoproject.org. It would be useful to alter the > report server as needed without adding flags. I don't understand. Why command line flags aren't suitable? The problem with environment variables is lack of transparency; they alter the behaviour in ways that are difficult to diagnoze. Command line options tend to be more explicit and easy to discover and understand. Alex
On Tue, 8 Apr 2025 at 13:24, 정재윤/Task Leader/SW Platform(연)선행Platform개발실 시스템SW Task <jaeyoon.jung@lge.com> wrote: > I agree that command line options are more explicit. I didn't mean that they aren't suitable. > But there's a case where adding a command line option isn't easy to the existing infra code and using an environment variable is considered as an alternative. Hello, I'd like to understand this a bit better. What in particular makes adding a command line option complicated, compared to adding an environment variable setting? Alex
On Wed, 9 Apr 2025 at 00:27, 정재윤/Task Leader/SW Platform(연)선행Platform개발실 시스템SW Task <jaeyoon.jung@lge.com> wrote: > Okay. Let me give you an example from our particular use case. > Suppose you set up a Jenkins CI with an automated error report when a build job gets failed, and you've also configured a script for that using send-error-report in Jenkins which is used in common for all multiple Jenkins nodes. There might be a case where you want to gather error reports in different servers for a particular maintenance purpose. > If the command line option is the only way to alter server URLs, then you will have to configure and maintain different script command lines for nodes affected. Environment variables can be useful this case, and I think it is more flexible than having diverged script command lines. But you don't have to hardcode the server value directly in the script that calls send-error-report. You can obtain it from the same environment variable, e.g. if the script is written in shell: send-error-report -s $ERROR_REPORT_SERVER Alex
On Wed, 9 Apr 2025 at 15:24, 정재윤/Task Leader/SW
Platform(연)선행Platform개발실 시스템SW Task <jaeyoon.jung@lge.com> wrote:
> Yes, that's possible too. But I have to make a command line change on my Jenkins which is still a less preferred option than environment variables. One more issue is that it is not applicable if you have https:// and http:// servers mixed where the latter requires additional --no-ssl flag. I think environment variables are useful when dealing with such variations.
The --no-ssl issue seems like a different issue that should be split
into a separate commit and reviewed on its own? Can you please do
that?
Otherwise, yes, you have to change the invocation of the tool to
include the option, and set the value from unix environment, if you
must. We can't start adding environment variables for anything and
everything that is configurable. You want --server, someone else will
then want --email, or --name, or something in some other tool they
use.
Alex
diff --git a/scripts/send-error-report b/scripts/send-error-report index cfbcaa52cb..3a522a3a5f 100755 --- a/scripts/send-error-report +++ b/scripts/send-error-report @@ -65,7 +65,7 @@ def edit_content(json_file_path): def prepare_data(args): # attempt to get the max_log_size from the server's settings - max_log_size = getPayloadLimit(args.protocol+args.server+"/ClientPost/JSON") + max_log_size = getPayloadLimit(args.server+"/ClientPost/JSON") if not os.path.isfile(args.error_file): log.error("No data file found.") @@ -135,9 +135,9 @@ def send_data(data, args): headers={'Content-type': 'application/json', 'User-Agent': "send-error-report/"+version} if args.json: - url = args.protocol+args.server+"/ClientPost/JSON/" + url = args.server+"/ClientPost/JSON/" else: - url = args.protocol+args.server+"/ClientPost/" + url = args.server+"/ClientPost/" req = urllib.request.Request(url, data=data, headers=headers) try: @@ -149,6 +149,23 @@ def send_data(data, args): print(response.read().decode('utf-8')) +def determine_server_url(args): + # Get the error report server from the environment variable + server = args.server or os.getenv('ERROR_REPORT_SERVER', 'errors.yoctoproject.org') + + # The scheme contained in the given URL takes precedence over --no-ssl flag + scheme = args.protocol + if server.startswith('http://'): + server = server[len('http://'):] + scheme = 'http://' + elif server.startswith('https://'): + server = server[len('https://'):] + scheme = 'https://' + + # Construct the final URL + return f"{scheme}{server}" + + if __name__ == '__main__': arg_parse = argparse_oe.ArgumentParser(description="This scripts will send an error report to your specified error-report-web server.") @@ -164,8 +181,7 @@ if __name__ == '__main__': arg_parse.add_argument("-s", "--server", help="Server to send error report to", - type=str, - default="errors.yoctoproject.org") + type=str) arg_parse.add_argument("-e", "--email", @@ -195,10 +211,10 @@ if __name__ == '__main__': dest="protocol", action="store_const", const="http://", default="https://") - - args = arg_parse.parse_args() + args.server = determine_server_url(args) + if (args.json == False): print("Preparing to send errors to: "+args.server)