Implement Relative Time for Outage Start and End Dates #78

KimFarida · 2024-03-28T22:08:15Z

Title: Implement Relative Time for Outage Start and End Dates

Description:

This pull request introduces the following improvements to the infra_status command:

Relative Time Display: Start and end times for ongoing and planned outages are now displayed in a human-readable relative format (e.g., "Just Now", "2 hours ago", "1 year ago") instead of absolute timestamps.
Improved Code Readability: Additional helper functions, date_validation and get_relative_time, are added to enhance code organization and maintainability.
Error Handling: The code now handles cases where outage retrieval from the Fedora Status API might fail (marked as pytest.mark.xfail in the test).

Before:

# ... (existing code)
    message = message + f"   Started at: {outage['startdate']}{NL}"
    message = message + (
        f"   Estimated to end: "
        f"{outage['enddate'] if outage['enddate'] else 'Unknown'}{NL}"
    )

After:

# ... (existing code)
    start_time = self.date_validation(outage.get("startdate"))
    end_time = (
        self.date_validation(outage.get("enddate"))
        if outage.get("enddate") is not None
        else None
    )

    now = datetime.datetime.now(start_time.tzinfo)
    delta_time = now - start_time

    relative_start_time = self.get_relative_time(delta_time)
    relative_end_time = (
        self.get_relative_time(end_time - now) if end_time is not None else None
    )

    message += f" * {format_title(outage)}{NL}"
    message += f"   Started : {relative_start_time}{NL}{start_time}"

    message += (
        f"   Estimated to end: "
        f"{end_time if end_time else 'Unknown'}{NL}"
        f"{relative_end_time  if relative_end_time else 'Unknown'}{NL}"
    )

Testing:

Unit tests are included to verify the functionality with various outage scenarios (provided in the code section).

Additional Notes:

This PR incorporates the code formatting changes (Black) and corrected docstrings mentioned in the previous commits.

I welcome your feedback and look forward to merging this PR!

…esolves fedora-infra#51) This commit updates the help text for the user info and user hello commands within the FasHandler class. The descriptions are modified to accurately reflect the information provided by each command: user info: Returns detailed information about a Fedora user, including username, human name, pronouns, creation date, timezone, locale, and GPG key IDs. user hello: Returns a brief introduction of a Fedora user, including username, human name, and pronouns (if available). This clarifies the distinction between the commands and improves the user experience when interacting with the bot. Signed-off-by: [KimFarida] <[[email protected]]>

KimFarida · 2024-03-29T20:00:02Z

@abompard @ryanlerch Hi! Could you please take a look

jeremycline

I would recommend a few adjustments, but this is a great start!

fedora/infra.py

jeremycline · 2024-04-01T15:27:54Z

fedora/infra.py

-                    )
+            message += f"**no planned or ongoing outages on Fedora Infrastructure.**{NL}"
+            await evt.respond(message)
+            return


Nice ✨

Thank you !

jeremycline · 2024-04-01T15:31:50Z

fedora/infra.py

+                )
+
+                message += f" * {format_title(outage)}{NL}"
+                message += f"   Started : {relative_start_time}{NL}{start_time}"


I assume the {NL} means "newline"?

Perhaps formatting it so it reads Started: 2024-04-01-11:30:38 (3 minutes ago) would be clearer.

jeremycline · 2024-04-01T15:35:05Z

fedora/infra.py

+                    relative_start_time = self.get_relative_time(start_time - now)
+
+                    message += (
+                        f"   Scheduled to start :" + f"{start_time}" + f"{relative_start_time}{NL}"


Suggested change

f" Scheduled to start :" + f"{start_time}" + f"{relative_start_time}{NL}"

f" Scheduled to start: {start_time} ({relative_start_time}){NL}"

jeremycline · 2024-04-01T15:36:49Z

fedora/infra.py

+                    message += (
+                        f"   Estimated to end: "
+                        f"{outage.get('enddate') if outage.get('enddate') else 'Unknown'}{NL}"
+                        f"{self.get_relative_time(outage.get('enddate') - now  if outage.get('enddate') else datetime.datetime.max - now)}{NL}"


Today I learned about datetime.datetime.max

fedora/infra.py

jeremycline · 2024-04-01T15:41:51Z

fedora/infra.py

+        date_string = datetime.datetime.strptime(date_string, date_format)
+        return date_string
+
+    def get_relative_time(self, delta):


It looks like the project already has a dependency on the arrow library, which provides a humanize call to calculate relative times. I'd recommend using that instead of implementing it yourself (although your implementation looks nice and simple!)

It looks like the project already has a dependency on the arrow library, which provides a humanize call to calculate relative times. I'd recommend using that instead of implementing it yourself (although your implementation looks nice and simple!)

inserts crying emoji I knew there was something i could use but the DSA warrior in me said no. It took a while to come up with the logic hahah. Thank you for the review!

jeremycline · 2024-04-01T15:43:59Z

tests/test_infra_status.py

@@ -31,8 +32,8 @@
    "outages",
    [
        (
-            [OUTAGE_FULL, OUTAGE_NO_ENDDATE, OUTAGE_NO_ENDDATE_NO_TICKET, OUTAGE_NO_TICKET],
-            [OUTAGE_FULL, OUTAGE_NO_ENDDATE, OUTAGE_NO_ENDDATE_NO_TICKET, OUTAGE_NO_TICKET],
+            [OUTAGE_FULL, OUTAGE_NO_ENDDATE, OUTAGE_NO_TICKET, OUTAGE_NO_ENDDATE_NO_TICKET],


Was there a reason to re-order things here? If it's just for style reasons I'd recommend to leave the change out and submit it as a separate PR so it's clear why the change occurred.

jeremycline · 2024-04-01T15:47:49Z

tests/test_infra_status.py

+                "##### Ongoing\n"
+                "● **thetitle (https://i.test/1)**\n"
+                "   Started at: 2023-08-06T1:00:00+0000\n"
+                "   Estimated to end: 2023-08-06T2:00:00+0000\n"


Will this test pass now that it should include relative times?

A library I've used in the past is https://pypi.org/project/freezegun/ to set a stable time to make it easier to write tests that do time calculations off the current time, in case that's why this doesn't include relative time changes.

…into infrastat-time-relative

KimFarida and others added 4 commits March 25, 2024 01:15

Merge branch 'fedora-infra:main' into main

0907557

Fix: Explicit Timezone Null Check for user oncall

09474ba

Feat: Added Relative Time to infra status| Refactored Tests

a306657

jeremycline reviewed Apr 1, 2024

View reviewed changes

KimFarida and others added 3 commits April 1, 2024 22:36

Rename date_validation helper function to format_date

1c781f5

Merge branch 'main' into infrastat-time-relative

e0c69fd

Merge branch 'main' of https://github.com/fedora-infra/maubot-fedora …

f5dcce5

…into infrastat-time-relative

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Relative Time for Outage Start and End Dates #78

Implement Relative Time for Outage Start and End Dates #78

KimFarida commented Mar 28, 2024

KimFarida commented Mar 29, 2024

jeremycline left a comment

jeremycline Apr 1, 2024

KimFarida Apr 1, 2024

jeremycline Apr 1, 2024

jeremycline Apr 1, 2024

KimFarida Apr 1, 2024

jeremycline Apr 1, 2024

jeremycline Apr 1, 2024

KimFarida Apr 1, 2024

jeremycline Apr 1, 2024

jeremycline Apr 1, 2024

	f" Scheduled to start :" + f"{start_time}" + f"{relative_start_time}{NL}"
	f" Scheduled to start: {start_time} ({relative_start_time}){NL}"

Implement Relative Time for Outage Start and End Dates #78

Are you sure you want to change the base?

Implement Relative Time for Outage Start and End Dates #78

Conversation

KimFarida commented Mar 28, 2024

KimFarida commented Mar 29, 2024

jeremycline left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment