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

use variables from varenv for delay / rate #30

Open
glelouet opened this issue Feb 15, 2024 · 5 comments
Open

use variables from varenv for delay / rate #30

glelouet opened this issue Feb 15, 2024 · 5 comments
Labels
enhancement good first issue A good issue for beginners

Comments

@glelouet
Copy link

I think using variables is way more powerful

Since the jar execution is in the guide, it can also be modified to show how scheduling can be parametrized

I would use expressions this way :

	@Value("${schedulingtasks.report.skip:false}")
	private boolean skipReport;

	@Scheduled(fixedRateString = "${schedulingtasks.report.period:5000}", initialDelayString = "${schedulingtasks.report.delay:0}")
	public void reportCurrentTime() {
		if(skipReport) return;

And would change the invocation to have the delay modified to 10s with

java -jar target/gs-scheduling-tasks-0.1.0.jar -Dschedulingtasks.report.delay=10000

(I guess this is how to transmit the variable with jar)

@robertmcnees robertmcnees added enhancement good first issue A good issue for beginners labels May 22, 2024
@robertmcnees
Copy link
Contributor

Hi @glelouet. Thanks for the issue. You have a point that using variables for the scheduling opens up more use cases. For this guide, I'm trying to think of the right balance between providing the most basic functionality to demonstrate scheduling a task while also giving the user other places to look for further improvements.

I think adding more information in the NOTE may be appropriate. Perhaps something like adding the bold/italic text below:

This example uses fixedRate(), which specifies the interval between method invocations, measured from the start time of each invocation. If you want to specify the interval as a parameter, you can use fixedRateString instead.

Regarding changes to the code and how to execute the JAR, I just pushed a PR #34 that would make that suggestion more complex. There are now 4 ways to run the application, and I'd prefer not to have a command line argument as part of those commands. Someone new to Spring may not understand the -D option without further explanation. I think for those reasons I'd rather just have a bit of text with a link to more information if the user wants to dive deeper.

I'll keep this issue open for a community contribution. PRs are always welcome!

@glelouet
Copy link
Author

hmm I would still write a working example, even if in the note.

IMO being able to provide working service with adaptable configuration is the key point in spring, so it's better to always have it available.

That being said, it's just my opinion based on my own experience and I understand that you disagree with it. As long as you took the time to consider it I'm fine with your choice.

I would make a PR if either

  1. you consider it's worth integrating
  2. you need me to provide more explanations

In that case I believe it's not worth (was only discussion) and you don't need more details.

I can still provide one if you want.

@robertmcnees
Copy link
Contributor

I agree external configuration is a really important part of Spring. My hesitation is based on keeping the guide as simple as possible and teaching only one thing at a time.

Regarding this specific change, I think a sentence in the note is appropriate but changing the code or showing a command line argument would generate more questions than value. For example, passing a command line argument is only one of many ways that someone could inject that value. Should we consider others? What if the user runs the code from their IDE? Should we explain defaults and/or how to pass a parameter through an IDE? Additionally, running a jar is one of 4 ways that this guide walks someone through running the application. Should all of those be updated? The desire is to keep this section as common as possible so someone who returns to multiple guides has a muscle memory for how to run their application.

I do think you've identified a potential gap in our guides documentation around external configuration and perhaps the use of profiles as well. I think a new guide on only that topic would be more appropriate, where we could discuss the issue in more detail and provide links to the reference docs. We have a guide on centralized configuration, but it focuses on a slightly different angle. You can request a new guide by opening an issue here. Details on writing a new guide are in the wiki.

For this issue I'll leave it open because I think there is value calling out fixedRateString as another option in the note, as I described above. Since it is a defined, one line documentation change I'll leave the 'good first issue' tag on as well.

If you'd like to submit a PR here, please feel free. If you decide to create an issue for a new guide link this conversation for context. I appreciate the discussion.

@glelouet
Copy link
Author

I don't think "how to parameter a spring-based app" is the scope of that guide.
That guide is "how to schedule tasks".

Setting the parameters is only present as a way to test the effective usage of that parameter in the app.
When I say "write a working example", I only mean have the parameters avail and changed somewhere (fixedvaluestring).

Wether it's by setting application.properties / yaml , creating a dedicated config for tests, setting the -Dx.y.z=10 in the IDE command line, setting up an autoconfig to load config beans into the properties unless provided by another module … it's up to the guide writer to choose one way to show it.
I gave an example of -D because that's what I use when I want to make tests in the IDE.

I think once I found out you can parameter the rates and delay, I did not use the non-parametrized one ever.

@glelouet
Copy link
Author

@robertmcnees I added an example in the PR.
Another method is scheduled with parameters controlling the skipping of the task, the rate, the delay with sensible default values.
The variables are set in the "config" file test, that is the "src/test/resources/application-config.yaml" file.
This file contains variables assignment with the skip commented to avoid not showing anything on the logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement good first issue A good issue for beginners
Projects
None yet
Development

No branches or pull requests

2 participants