user25651: I've got a few functions like this that log, or do more critical things like get/save configuration variables.
/**
   Write weather data to csv
   commit: datetime,minHumidity,maxHumidity,minTemperature,maxTemperature,rainfall,irrigationRan
*/
void writeWeatherData() {
  noInterrupts();
  DBG_OUTPUT.println(F("[WEATHER DATA] Attempting to save weather data..."));
  bool writeHeader = SPIFFS.exists("/weather_data.csv") ? false : true;
  File file = SPIFFS.open("/weather_data.csv", "a");
  if (!file) {
    addPrintError("[WEATHER DATA] There was an error opening weather_data.csv for writing.");
    interrupts();
    return;
  }
  String data = "";
  if (writeHeader) {
    data += "datetime,minHumidity,maxHumidity,minTemperature,maxTemperature,rainfall,irrigationRan\r\n";
  }
  DateTime now = rtc.now();
  char buf[] = "MM/DD/YY hh:mm:ss";
  String t = now.toString(buf);
  float rainfall = getRainfall();
  data += t + "," + minHumidity + "," + maxHumidity + "," + minTemperature + "," + maxTemperature + "," + rainfall + "," + irrigationRan + "\r\n";
  if (file.print(data)) {
    DBG_OUTPUT.println(F("[WEATHER DATA] File was written."));
  } else {
    addPrintError("[WEATHER DATA] File write failed.");
  }
  file.close();
  DBG_OUTPUT.println(F("[WEATHER DATA] Task completed."));
  interrupts();
}

I surround  code with noInterrupts and interrupts however I am not sure if it is required. Assume that an interrupt is triggered at the exact moment when the above function is saving something, what exactly happens if I didn't have noInterrupts .etc.?

user7508: John Romkey provided you an excellent answer. I am adding this just to
provide another perspective, hopefully complementary.
Interrupts are meant to handle the most time-critical tasks. Those tasks
that cannot wait for the next loop() iteration, such as counting a
pulse from an encoder, or getting a byte out of the UART receive buffer.
If you delay them too much, they may miss their deadline, resulting in
unreliable operation of the whole program (counter missing pulses, UART
missing bytes...).
Sometimes you have a piece of code that cannot afford being interrupted.
Maybe it is extremely time-critical, like the generation in software of
a pulse that is exactly 0.5 µs wide. More commonly it will be
reading data from memory that is being modified by an interrupt handler,
and you don't want that data to be modified in the middle of the read
operation. Such pieces of code are called critical sections, and you
keep them safe in between noInterrupts() and interrupts(). If an
interrupt request fires while the program is running a critical section,
the request is put on hold and serviced only when the critical section
is done. This adds some latency to the interrupt which, if
excessive, can lead to the interrupt missing its deadline. This is the
reason critical sections should be kept as short as possible.
You have been told that interrupt handlers should be kept as short as
possible, but I am not sure you know why it is so. The fundamental
reason is that interrupt handlers are themselves critical sections.
While a handler is running, other interrupts are blocked. On some
architectures (notably not AVR, I don't know about the ESP), only
interrupts of lower priority are blocked, but it is still an issue. The
rationale for keeping those handlers short applies equally to any
critical section.
Now, just to provide an example of the sort of bad things that can
happen if you block interrupts unnecessarily, consider this piece of
code:
noInterrupts();
Serial.println("Inside critical section.");
interrupts();

What this does is put the string "Inside critical section." in a
software buffer. Not a huge task you may say. Actually pushing the bytes
out to the UART is the job of an interrupt handler, triggered by the
UART when it is ready to accept a new byte. But then what happens if
there is not enough free space in the buffer for the string? In this
case, Serial.println() waits in a busy loop. It waits for the
interrupt to move bytes out of the buffer and make space for the new
string. But wait, we have just disabled interrupts... See the problem?
This is the reason it is generally advised to never Serial.print()
inside an interrupt handler. The same applies to any critical section.
Edit: As Juraj points out in a comment, the HardwareSerial code
now implements a workaround against this problem: if it
detects it has to wait with interrupts disabled, it takes care of
calling the interrupt handler itself. This is a (relatively) late
addition to the HardwareSerial code. Their authors presumably
witnessed too many beginners doing debug prints from interrupt handlers
and being bitten by this issue. Note that it is a workaround, not a
solution: it will often make the critical section last for an
unreasonably long time.
I wouldn't rely on similar APIs, or even on other cores, systematically
implementing this kind of safeguards though: some library authors just
trust the user to not do silly things.

user46852: Interrupts happen all the time and code works just fine without locking them out. It doesn't matter if it's at the exact moment that a function is saving something - a properly written interrupt handler will save any necessary state so that whatever was interrupted can continue without problems. If it doesn't, the software will crash hard.
You should lock out interrupts as little as possible. Interrupts are used for critical timing functions and for network and other communications. Locking them out will cause these functions to be unreliable and may disrupt network connections.
The only time you should disable interrupts is when a data structure that an interrupt handler needs to use is in an inconsistent state. If an interrupt handler needs to use a shared data structure, disable interrupts before beginning to modify it outside of the interrupt handler, and re-enable them when you're done. Make sure it's as brief as possible.
Interrupt handlers should do as little as possible. They definitely should not call SPIFFS, should not call the real time clock, should not allocate or instantiate objects and should not do i/o operations. And you should not be disabling interrupts around calls to SPIFFS in hopes of making it safe for your interrupt handler to also call SPIFFS.
Take this as an example: suppose you want to use SPIFFS in both an interrupt handler and outside the interrupt handler. So you think you'll be clever and disable interrupts before you call SPIFFS outside the interrupt handler. Do you know exactly how SPIFFS works? Do you know whether SPIFFS uses any other functions that you might need to disable interrupts for in other parts of your code so that the interrupt handler won't disrupt them? Do you know for certain that SPIFFS doesn't allocate memory temporarily? Imagine that it does - now you also need to find every place in your code that might allocate memory and lock out interrupts around it, too. And maybe SPIFFS doesn't allocate memory - but do you know that SPIFFS in the future will never allocate memory?
You can't make assumptions about exactly how modules that you call are implemented, or how they'll be implemented in the future.
Unless you seriously know what you're doing, on microcontrollers like Arduinos, the ESP8266 and the ESP32, the best thing an interrupt handler can do is just set a flag to indicate that the interrupt occurred, and let code in the loop() check for the flag. The flag should most likely just be a simple Boolean variable declared as volatile so that the C compiler is aware that its value may suddenly change:
volatile boolean interrupt_flag = false;

It's safe to set that interrupt_flag to true inside the interrupt handler and it's safe to check the value in loop(), reset it to false and do whatever work was needed. Don't do more than this unless you know what you're doing.

25651: <p>I've got a few functions like this that log, or do more critical things like get/save configuration variables.</p>

<pre><code>/**
   Write weather data to csv
   commit: datetime,minHumidity,maxHumidity,minTemperature,maxTemperature,rainfall,irrigationRan
*/
void writeWeatherData() {
  noInterrupts();
  DBG_OUTPUT.println(F("[WEATHER DATA] Attempting to save weather data..."));
  bool writeHeader = SPIFFS.exists("/weather_data.csv") ? false : true;
  File file = SPIFFS.open("/weather_data.csv", "a");
  if (!file) {
    addPrintError("[WEATHER DATA] There was an error opening weather_data.csv for writing.");
    interrupts();
    return;
  }
  String data = "";
  if (writeHeader) {
    data += "datetime,minHumidity,maxHumidity,minTemperature,maxTemperature,rainfall,irrigationRan\r\n";
  }
  DateTime now = rtc.now();
  char buf[] = "MM/DD/YY hh:mm:ss";
  String t = now.toString(buf);
  float rainfall = getRainfall();
  data += t + "," + minHumidity + "," + maxHumidity + "," + minTemperature + "," + maxTemperature + "," + rainfall + "," + irrigationRan + "\r\n";
  if (file.print(data)) {
    DBG_OUTPUT.println(F("[WEATHER DATA] File was written."));
  } else {
    addPrintError("[WEATHER DATA] File write failed.");
  }
  file.close();
  DBG_OUTPUT.println(F("[WEATHER DATA] Task completed."));
  interrupts();
}
</code></pre>

<p>I surround  code with <code>noInterrupts</code> and <code>interrupts</code> however I am not sure if it is required. Assume that an interrupt is triggered at the exact moment when the above function is saving something, what exactly happens if I didn't have <code>noInterrupts</code> .etc.?</p>
 7508: <p>John Romkey provided you an <em>excellent</em> answer. I am adding this just to
provide another perspective, hopefully complementary.</p>

<p>Interrupts are meant to handle the most time-critical tasks. Those tasks
that cannot wait for the next <code>loop()</code> iteration, such as counting a
pulse from an encoder, or getting a byte out of the UART receive buffer.
If you delay them too much, they may miss their deadline, resulting in
unreliable operation of the whole program (counter missing pulses, UART
missing bytes...).</p>

<p>Sometimes you have a piece of code that cannot afford being interrupted.
Maybe it is extremely time-critical, like the generation in software of
a pulse that is exactly 0.5&nbsp;µs wide. More commonly it will be
reading data from memory that is being modified by an interrupt handler,
and you don't want that data to be modified in the middle of the read
operation. Such pieces of code are called <em>critical sections</em>, and you
keep them safe in between <code>noInterrupts()</code> and <code>interrupts()</code>. If an
interrupt request fires while the program is running a critical section,
the request is put on hold and serviced only when the critical section
is done. This adds some <strong>latency</strong> to the interrupt which, if
excessive, can lead to the interrupt missing its deadline. This is the
reason critical sections should be kept as short as possible.</p>

<p>You have been told that interrupt handlers should be kept as short as
possible, but I am not sure you know <em>why</em> it is so. The fundamental
reason is that <em>interrupt handlers are themselves critical sections</em>.
While a handler is running, other interrupts are blocked. On some
architectures (notably <em>not</em> AVR, I don't know about the ESP), only
interrupts of lower priority are blocked, but it is still an issue. The
rationale for keeping those handlers short applies equally to <em>any</em>
critical section.</p>

<p>Now, just to provide an example of the sort of bad things that can
happen if you block interrupts unnecessarily, consider this piece of
code:</p>

<pre class="lang-cpp prettyprint-override"><code>noInterrupts();
Serial.println("Inside critical section.");
interrupts();
</code></pre>

<p>What this does is put the string <code>"Inside critical section."</code> in a
software buffer. Not a huge task you may say. Actually pushing the bytes
out to the UART is the job of an interrupt handler, triggered by the
UART when it is ready to accept a new byte. But then what happens if
there is not enough free space in the buffer for the string? In this
case, <code>Serial.println()</code> waits in a busy loop. It waits for the
interrupt to move bytes out of the buffer and make space for the new
string. But wait, we have just disabled interrupts... See the problem?
This is the reason it is generally advised to never <code>Serial.print()</code>
inside an interrupt handler. The same applies to any critical section.</p>

<p><strong>Edit</strong>: As Juraj points out in a comment, the <code>HardwareSerial</code> code
now <a href="https://github.com/arduino/ArduinoCore-avr/commit/dbe23685c27cb1467dc84140e4899a0bf8e23248" rel="nofollow noreferrer">implements a workaround against this problem</a>: if it
detects it has to wait with interrupts disabled, it takes care of
calling the interrupt handler itself. This is a (relatively) late
addition to the <code>HardwareSerial</code> code. Their authors presumably
witnessed too many beginners doing debug prints from interrupt handlers
and being bitten by this issue. Note that it is a workaround, not a
solution: it will often make the critical section last for an
unreasonably long time.</p>

<p>I wouldn't rely on similar APIs, or even on other cores, systematically
implementing this kind of safeguards though: some library authors just
trust the user to not do silly things.</p>
 46852: <p>Interrupts happen all the time and code works just fine without locking them out. It doesn't matter if it's at the exact moment that a function is saving something - a properly written interrupt handler will save any necessary state so that whatever was interrupted can continue without problems. If it doesn't, the software will crash hard.</p>

<p>You should lock out interrupts as little as possible. Interrupts are used for critical timing functions and for network and other communications. Locking them out will cause these functions to be unreliable and may disrupt network connections.</p>

<p>The only time you should disable interrupts is when a data structure that an interrupt handler needs to use is in an inconsistent state. If an interrupt handler needs to use a shared data structure, disable interrupts before beginning to modify it outside of the interrupt handler, and re-enable them when you're done. Make sure it's as brief as possible.</p>

<p>Interrupt handlers should do as little as possible. They definitely should not call SPIFFS, should not call the real time clock, should not allocate or instantiate objects and should not do i/o operations. And you should not be disabling interrupts around calls to SPIFFS in hopes of making it safe for your interrupt handler to also call SPIFFS.</p>

<p>Take this as an example: suppose you want to use SPIFFS in both an interrupt handler and outside the interrupt handler. So you think you'll be clever and disable interrupts before you call SPIFFS outside the interrupt handler. Do you know exactly how SPIFFS works? Do you know whether SPIFFS uses any other functions that you might need to disable interrupts for in other parts of your code so that the interrupt handler won't disrupt them? Do you know for certain that SPIFFS doesn't allocate memory temporarily? Imagine that it does - now you also need to find every place in your code that might allocate memory and lock out interrupts around it, too. And maybe SPIFFS doesn't allocate memory - but do you know that SPIFFS in the future will never allocate memory?</p>

<p>You can't make assumptions about exactly how modules that you call are implemented, or how they'll be implemented in the future.</p>

<p>Unless you seriously know what you're doing, on microcontrollers like Arduinos, the ESP8266 and the ESP32, the best thing an interrupt handler can do is just set a flag to indicate that the interrupt occurred, and let code in the <code>loop()</code> check for the flag. The flag should most likely just be a simple Boolean variable declared as volatile so that the C compiler is aware that its value may suddenly change:</p>

<pre><code>volatile boolean interrupt_flag = false;
</code></pre>

<p>It's safe to set that <code>interrupt_flag</code> to <code>true</code> inside the interrupt handler and it's safe to check the value in <code>loop()</code>, reset it to <code>false</code> and do whatever work was needed. Don't do more than this unless you know what you're doing.</p>
