fix: Update Support Window spreadsheet #414

MimmyJau · 2023-07-10T20:17:45Z

Note: There are two commits. The first commit adds the release date and EOL dates for the current ElasticSearch 7.17 version. The second commit adds functionality for auto-updating some of the version numbers based on Tutor defaults.

Background: Based on the notes in this issue. Current Support Windows spreadsheet is not up-to-date. Noticed there hasn't been any attempt to add these features, so this is a first pass at trying to update barcalendar.py.

Doesn't include:

tests
auto-updating django, ubuntu, ruby, or node versions

Happy to make any changes based on feedback. Thanks in advance.

openedx-webhooks · 2023-07-10T20:17:50Z

Thanks for the pull request, @MimmyJau! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

mphilbrick211 · 2023-07-11T14:28:08Z

Hi @MimmyJau! Thanks for this contribution. Please let me know if you have any questions regarding filling out the CLA form.

MimmyJau · 2023-07-11T16:01:12Z

Hi @MimmyJau! Thanks for this contribution. Please let me know if you have any questions regarding filling out the CLA form.

Thanks! I've submitted the CLA, just waiting now for it to be processed.

e0d · 2023-07-12T16:43:34Z

I notice there are some commit-lint failures. Please note that we use conventional commits across Open edX projects. You can read about the details here. Can you please amend your commit messages to follow our standard?

MimmyJau · 2023-07-12T16:53:31Z

Thanks @e0d! I noticed I made that mistake but was a little cautious to force push an updated git history.

With your permission I'll go ahead and make those changes.

e0d · 2023-07-12T16:56:21Z

No worries, you can force push in this case.

nedbat · 2023-07-17T01:41:33Z

barcalendar.py

+    Get openedx version name in line from YAML file.
+    e.g. "open-release/palm.1" -> "Palm"
+    """
+    match = re.search(r'(?P<version_name>[A-Za-z]+)[\.\d+]*$', line)


I'm not sure how this setting is managed. Sometimes its value is open-release/palm.master. Perhaps the best approach is to use:

match = re.search(r'/(?P<version_name>[A-Za-z]+)\.', line)

Modified it to

match = re.search(r'(?P<version_name>[A-Za-z]+)(\.\w*)*$', line)

to also match the following cases:

"open-release/palm.1"

"open-release/palm.1.2.3"

"open-release/palm.master"

"open-release/palm.master.1.2.3"

"open-release/palm."

It is a bit redundant. Looking at this page, it shouldn't deviate from the two formats we considered here.

Happy to go with the simpler approach.

To me, it's simpler to say, "the word between the slash and the dot." Your regex is, "the word followed by dots and words at the end of the line," which seems more confusing.

When you phrase it that way, I completely agree. I've made the change.

barcalendar.py

nedbat · 2023-07-17T01:43:59Z

barcalendar.py

+
+    if resp.status_code == 200:
+        return safe_load(resp.text)
+    return None


For many of the info functions, it would be better to raise an exception than to return None. If a problem occurs, and they return None, we'll have a mysterious error elsewhere in the program.

Added a raise RuntimeError if resp.status_code is anything but 200.

Also in the except block above, I re-raised the RequestException error. Let me know if this makes sense.

Requests has a helper method: https://requests.readthedocs.io/en/latest/api/#requests.Response.raise_for_status . It will raise an exception based on the status code.

Thanks, did not know about this. I've incorporated it.

barcalendar.py

See here for original comment: openedx#414 (comment) Version name might be something like "palm.master", in which case we don't just want to capture a version number.

Was previously importing a specific function but this was not consistent with style. See here for original comment: openedx#414 (comment)

Before, if http request to tutor's defaults.yml returned status code != 200, function would return None and continue. No error would be raised. This fix now raises a RuntimError. See here for original comment: openedx#414 (comment)

f-string is unnecessary if the string is a single Python expression. Replaced f-string with just the expression where possible. See here for original comment: openedx#414 (comment)

mphilbrick211 · 2023-07-17T23:00:49Z

Hi @nedbat! Would you please re-run the tests, as they bounced back to needing approval.

barcalendar.py

nedbat · 2023-07-18T15:56:47Z

barcalendar.py

+    if match is not None:
+        version_number = match.group("version_number")
+        return version_number
+    return None


I think it would be good to raise an exception if the version number can't be found. If it's missing, then no error happens, and the chart is made with no version highlighted as current. Maybe something like:

Suggested change

return None

raise ValueError(f"Couldn't get version from {line!r}")

and something similar for parse_version_name also.

Makes sense and I agree. The latest commit adds this and also an exception for the related function parse_version_name.

nedbat · 2023-07-18T15:58:24Z

Sorry for all the tweaks, thanks for pushing through with this.

BTW, I have to ask: what drew your attention to this program? It's off the beaten path, so to speak! 😄

Add support date for Elasticsearch version 7.17. Did not include support dates for previous versions between 7.14 and 7.16 inclusive.

Add functions for automatically pulling version number from tutor's default config file and choosing which bars to bold and outline. This pulls the name and / or number for the current version of openedx, MongoDB, Elasticsearch, MySQL, and Redis. It does not pull the current version of Python, Django, Ubuntu, Node, or Ruby. To be clear, this commit doesn't add new bars to the calendar, it only automates how we choose which bars to bold and outline. For example, if we were to switch to Elasticsearch 8.x.x, we'll still need to manually add the release and EOL dates.

See here for original comment: openedx#414 (comment) Version name might be something like "palm.master", in which case we don't just want to capture a version number.

Was previously importing a specific function but this was not consistent with style. See here for original comment: openedx#414 (comment)

Before, if http request to tutor's defaults.yml returned status code != 200, function would return None and continue. No error would be raised. This fix now raises a RuntimError. See here for original comment: openedx#414 (comment)

f-string is unnecessary if the string is a single Python expression. Replaced f-string with just the expression where possible. See here for original comment: openedx#414 (comment)

Co-authored-by: Ned Batchelder <[email protected]>

Before, if version name or number wasn't found, will continue to run program and generate a bar calendar that doesn't have bolded bars. Want to instead raise an error. See here for original comment: openedx#414 (comment)

MimmyJau · 2023-07-18T18:49:56Z

Sorry for all the tweaks, thanks for pushing through with this.

BTW, I have to ask: what drew your attention to this program? It's off the beaten path, so to speak! 😄

Not a problem at all! Thanks for taking the time and attention.

I came across the issue here. Saw that it was backlogged and at the top. It also looked like a good first issue so I figured why not :)

openedx-webhooks · 2023-07-18T20:15:25Z

@MimmyJau 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jul 10, 2023

MimmyJau mentioned this pull request Jul 10, 2023

Update the Support Windows spreadsheet openedx/wg-build-test-release#199

Closed

MimmyJau changed the title ~~fix: update Support Windsows spreadsheet~~ fix: update Support Windows spreadsheet Jul 11, 2023

mphilbrick211 added the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Jul 11, 2023

MimmyJau changed the title ~~fix: update Support Windows spreadsheet~~ fix: Update Support Windows spreadsheet Jul 11, 2023

MimmyJau changed the title ~~fix: Update Support Windows spreadsheet~~ fix: Update Support Windows Spreadsheet Jul 12, 2023

MimmyJau changed the title ~~fix: Update Support Windows Spreadsheet~~ fix: Update Support Windows spreadsheet Jul 12, 2023

MimmyJau force-pushed the update-barcalendar branch from 465134d to 1c33cc1 Compare July 12, 2023 17:12

MimmyJau changed the title ~~fix: Update Support Windows spreadsheet~~ fix: Update Support Window spreadsheet Jul 13, 2023

nedbat reviewed Jul 17, 2023

View reviewed changes

barcalendar.py Outdated Show resolved Hide resolved

nedbat reviewed Jul 17, 2023

View reviewed changes

barcalendar.py Outdated Show resolved Hide resolved

e0d removed the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Jul 17, 2023

MimmyJau added a commit to MimmyJau/openedx-repo-tools that referenced this pull request Jul 17, 2023

refactor: Import yaml library for consistency

2a45806

Was previously importing a specific function but this was not consistent with style. See here for original comment: openedx#414 (comment)

nedbat reviewed Jul 18, 2023

View reviewed changes

barcalendar.py Outdated Show resolved Hide resolved

nedbat reviewed Jul 18, 2023

View reviewed changes

MimmyJau added 2 commits July 18, 2023 08:58

fix: Update Elasticsearch support dates

cde0cb3

Add support date for Elasticsearch version 7.17. Did not include support dates for previous versions between 7.14 and 7.16 inclusive.

MimmyJau added 6 commits July 18, 2023 08:58

fix: Regex to match version name if suffix is word

28065b7

See here for original comment: openedx#414 (comment) Version name might be something like "palm.master", in which case we don't just want to capture a version number.

refactor: Import yaml library for consistency

dbecbc7

Was previously importing a specific function but this was not consistent with style. See here for original comment: openedx#414 (comment)

fix: Raise error if status code isn't 200

a3e7a3b

Before, if http request to tutor's defaults.yml returned status code != 200, function would return None and continue. No error would be raised. This fix now raises a RuntimError. See here for original comment: openedx#414 (comment)

refactor: Remove f-string for single expressions

99fc5fa

f-string is unnecessary if the string is a single Python expression. Replaced f-string with just the expression where possible. See here for original comment: openedx#414 (comment)

fix: Simplify regex for matching version name

0bb8db6

fix: Use requests' built-in method to raise exc

9b46aff

nedbat force-pushed the update-barcalendar branch from d402ab2 to 9b46aff Compare July 18, 2023 15:58

MimmyJau and others added 2 commits July 18, 2023 13:53

refactor: Separate third-party imports from built-in

cfae02f

Co-authored-by: Ned Batchelder <[email protected]>

fix: Raise error if version name/number not found

f98140b

Before, if version name or number wasn't found, will continue to run program and generate a bar calendar that doesn't have bolded bars. Want to instead raise an error. See here for original comment: openedx#414 (comment)

nedbat approved these changes Jul 18, 2023

View reviewed changes

nedbat merged commit 2dc7322 into openedx:master Jul 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Update Support Window spreadsheet #414

fix: Update Support Window spreadsheet #414

MimmyJau commented Jul 10, 2023

openedx-webhooks commented Jul 10, 2023 •

edited

Loading

mphilbrick211 commented Jul 11, 2023

MimmyJau commented Jul 11, 2023

e0d commented Jul 12, 2023

MimmyJau commented Jul 12, 2023

e0d commented Jul 12, 2023

nedbat Jul 17, 2023 •

edited

Loading

MimmyJau Jul 17, 2023

nedbat Jul 17, 2023

MimmyJau Jul 17, 2023

nedbat Jul 17, 2023

MimmyJau Jul 17, 2023

nedbat Jul 17, 2023

MimmyJau Jul 17, 2023

mphilbrick211 commented Jul 17, 2023

nedbat Jul 18, 2023 •

edited

Loading

MimmyJau Jul 18, 2023

nedbat commented Jul 18, 2023

MimmyJau commented Jul 18, 2023

openedx-webhooks commented Jul 18, 2023

	return None
	raise ValueError(f"Couldn't get version from {line!r}")

fix: Update Support Window spreadsheet #414

fix: Update Support Window spreadsheet #414

Conversation

MimmyJau commented Jul 10, 2023

openedx-webhooks commented Jul 10, 2023 • edited Loading

mphilbrick211 commented Jul 11, 2023

MimmyJau commented Jul 11, 2023

e0d commented Jul 12, 2023

MimmyJau commented Jul 12, 2023

e0d commented Jul 12, 2023

nedbat Jul 17, 2023 • edited Loading

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

mphilbrick211 commented Jul 17, 2023

nedbat Jul 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nedbat commented Jul 18, 2023

MimmyJau commented Jul 18, 2023

openedx-webhooks commented Jul 18, 2023

openedx-webhooks commented Jul 10, 2023 •

edited

Loading

nedbat Jul 17, 2023 •

edited

Loading

nedbat Jul 18, 2023 •

edited

Loading