Deprecated get_s3fs_session #768

Sherwin-14 · 2024-07-19T13:19:49Z

I have made the changes. Do we update all the instances of get_s3fs_session now or is that something to be decided later?

📚 Documentation preview 📚: https://earthaccess--768.org.readthedocs.build/en/768/

chuckwondo

@Sherwin-14, please read through the conversation again: #766

tl;dr: (1) you must deprecate get_s3fs_session, (2) the new name should be get_s3_filesystem

Sherwin-14 · 2024-07-19T14:08:15Z

@Sherwin-14, please read through the conversation again: #766

tl;dr: (1) you must deprecate get_s3fs_session, (2) the new name should be get_s3_filesystem

Oops I missed that! There are two methods of deprecation(if I am not wrong) which one should we choose here? Should we make a change in docstrings or should we add a deprecated decorator

chuckwondo · 2024-07-19T14:32:14Z

For full details on use of the @deprecated decorator, see https://peps.python.org/pep-0702/

In summary, here's what you need to do (with an example afterward):

In the imports section at the top of the file, add from typing_extensions import deprecated (or insert deprecated into an existing from typing_extensions import statement in the file)
copy each existing function signature and paste it above the existing one
rename the existing function
from the newly copied function (with the "old" name), call the renamed function
decorate the newly copied function with @deprecated("Use get_s3_filesystem instead")

Example:

In store.py we have the Store class with the following method (I've left out the implementation for brevity):

    def get_s3fs_session(
        self,
        daac: Optional[str] = None,
        concept_id: Optional[str] = None,
        provider: Optional[str] = None,
        endpoint: Optional[str] = None,
    ) -> s3fs.S3FileSystem:
        # Existing implementation

Here's how we want to change things:

    @deprecated("Use get_s3_filesystem instead")
    def get_s3fs_session(
        self,
        daac: Optional[str] = None,
        concept_id: Optional[str] = None,
        provider: Optional[str] = None,
        endpoint: Optional[str] = None,
    ) -> s3fs.S3FileSystem:
        # COPY OVER THE DOCSTRING HERE
        return self.get_s3_filesystem(daac, concept_id, provider, endpoint)

    def get_s3_filesystem(
        self,
        daac: Optional[str] = None,
        concept_id: Optional[str] = None,
        provider: Optional[str] = None,
        endpoint: Optional[str] = None,
    ) -> s3fs.S3FileSystem:
        # Existing implementation

You also need to add this import to store.py:

from typing_extensions import deprecated

mfisher87 · 2024-07-19T16:02:02Z

Neat, TIL! But I don't think @deprecated drops until 3.13?

EDIT: Nevermind :) Should have read this part:

Creating a new decorator poses no backwards compatibility concerns. As with all new typing functionality, the @deprecated decorator will be added to the typing_extensions module, enabling its use in older versions of Python.

chuckwondo

@Sherwin-14, there's a bit more to do than only deprecating and renaming the Store.get_s3fs_session method.

Please do a search throughout the entire codebase for occurrences of get_s3fs_session and determine the appropriate changes as follows:

Change all references to the Store.get_s3fs_session method to get_s3_filesystem so that within the codebase we're not using the newly deprecated method, but are instead using the newly introduced method that replaces it.
There is at least one other place where you must do the same thing you did in store.py. For example, in api.py, there is a top-level get_s3fs_session function. You must do the same thing with that function: deprecate it, copy it to a new function named get_s3_filesystem, and modify all of its references to earthaccess.__store__.get_s3fs_session to earthaccess.__store__.get_s3_filesystem.
Update references in kerchunk.py, test_auth.py, and test_store.py (and see if there are any other places that I may have missed listing here)

Please also add an appropriate entry to CHANGELOG.md, but before doing so, please make sure you have merged the latest changes from the main branch, as Luis just released v0.10.0, so he has updated CHANGELOG.md, so you want to pull in those changes before adding your own entry.

Once you have merged in the change made by Luis, you should add an entry above the new v0.10.0 heading that reads like so:

### Changed

- Deprecate `earthaccess.get_s3fs_session` and `Store.get_s3fs_session`. Use
  `earthaccess.get_s3_filesystem` and `Store.get_s3_filesystem`, respectively,
  instead ([#766](https://github.com/nsidc/earthaccess/issues/766))

In general, before opening a PR, you should locally run unit tests to make sure they pass. Further, after pushing up your work to your fork, but before opening a PR, you should check the success of the automated build. See https://github.com/Sherwin-14/earthaccess/actions/workflows/test.yml. If the build fails (because unit tests fail, for example), you should attempt to fix the failure.

Once you get things to work, or if you cannot determine how to fix things, you can then open a PR. In the case of opening the PR even when you cannot determine how to fix the broken build, you can ask for help to get things working.

earthaccess/store.py

Sherwin-14 · 2024-07-22T12:37:27Z

@chuckwondo Oh I guess there is some confusion here. Maybe I wasn't able to communicate what I felt after making a PR, all I was asking was that should we replace all the occurrences of get_s3fs_session at this point in time or maybe it something that we reserve for later. I am quite new to all of this, plus I didn't know that immediately after deprecation we have to replace all the instances ( maybe for some weird reason I thought this is something to be done later). Also I will try to follow the best practices from now on.

chuckwondo · 2024-07-22T12:55:36Z

@chuckwondo Oh I guess there is some confusion here. Maybe I wasn't able to communicate what I felt after making a PR, all I was asking was that should we replace all the occurrences of get_s3fs_session at this point in time or maybe it something that we reserve for later. I am quite new to all of this, plus I didn't know that immediately after deprecation we have to replace all the instances ( maybe for some weird reason I thought this is something to be done later). Also I will try to follow the best practices from now on.

No worries. I'm happy to help you through the process. I'm also happy for us to collaborate on this during the next earthaccess hackday, which is tomorrow, if you're able to attend. If you're not able to attend tomorrow, I'm happy to schedule a pairing session with you some other time this week.

chuckwondo · 2024-07-22T12:59:01Z

@Sherwin-14, we'll also want to add a unit test to make sure the deprecation warning is actually emitted. For reference: https://docs.python.org/3/library/warnings.html#testing-warnings

Sherwin-14 · 2024-07-23T07:03:15Z

Did all the changes in the respective files. I am also working on coming up with the unit test for emitting deprecation warning as well, would probably amend in the same commit.

Sherwin-14 · 2024-07-23T08:15:36Z

This is something I came up with for the unit test.

import warnings
import earthaccess 
from earthaccess import api 
from earthaccess.store import Store

def test_deprecation_warning():
    store = earthaccess.store.Store()
    with warnings.catch_warnings(record=True) as w:
        # Cause all warnings to always be triggered.
        warnings.simplefilter("always")
        # Trigger a warning.
        api.get_s3fs_session()
        store.get_s3fs_session()
        # Verify some things
        assert len(w) == 2
        assert issubclass(w[0].category, DeprecationWarning)
        assert "Use get_s3_filesystem instead" in str(w[0].message)
        assert issubclass(w[1].category, DeprecationWarning)
        assert "Use get_s3_filesystem instead" in str(w[1].message)

I ran into an error while running this test on my local machine which goes something like this FAILED tests/unit/test_deprecations.py::test_deprecation_warning - TypeError: Store.__init__() missing 1 required positional argument: 'auth'. I guess we need some access token here to be passed into the Store Class. What can be the next steps from here?

chuckwondo

Looking good! Just a couple of suggestions for adjusting the docstrings for the new functions. I'll reply separately to your question about the unit test you're working on.

earthaccess/api.py

earthaccess/store.py

chuckwondo · 2024-07-23T13:30:38Z

@Sherwin-14, nice work so far! The unit test all pass now.

@mfisher87, any idea why the integration tests are failing? I see that authentication is failing:

INFO     test_api:test_api.py:17 Current username: 
INFO     test_api:test_api.py:18 earthaccess version: 0.10.0
INFO     earthaccess.auth:auth.py:312 Authentication with Earthdata Login failed with:
{"error":"invalid_header","error_description":"Invalid header"}

The workflow pulls secrets from the github environment, but that first line of output above shows that the username is an empty string. I don't have access to the environments in the repo, and I don't know whose EDL credentials are being used in the secrets. Do you know?

chuckwondo · 2024-07-23T13:41:32Z

This is something I came up with for the unit test.

import warnings
import earthaccess 
from earthaccess import api 
from earthaccess.store import Store

def test_deprecation_warning():
    store = earthaccess.store.Store()
    with warnings.catch_warnings(record=True) as w:
        # Cause all warnings to always be triggered.
        warnings.simplefilter("always")
        # Trigger a warning.
        api.get_s3fs_session()
        store.get_s3fs_session()
        # Verify some things
        assert len(w) == 2
        assert issubclass(w[0].category, DeprecationWarning)
        assert "Use get_s3_filesystem instead" in str(w[0].message)
        assert issubclass(w[1].category, DeprecationWarning)
        assert "Use get_s3_filesystem instead" in str(w[1].message)

I ran into an error while running this test on my local machine which goes something like this FAILED tests/unit/test_deprecations.py::test_deprecation_warning - TypeError: Store.__init__() missing 1 required positional argument: 'auth'. I guess we need some access token here to be passed into the Store Class. What can be the next steps from here?

@Sherwin-14, nice job on coming up with the initial unit test for this. Will you be at today's hackday? If so, we can continue with this then.

Sherwin-14 · 2024-07-23T13:43:56Z

This is something I came up with for the unit test.
import warnings
import earthaccess 
from earthaccess import api 
from earthaccess.store import Store

def test_deprecation_warning():
    store = earthaccess.store.Store()
    with warnings.catch_warnings(record=True) as w:
        # Cause all warnings to always be triggered.
        warnings.simplefilter("always")
        # Trigger a warning.
        api.get_s3fs_session()
        store.get_s3fs_session()
        # Verify some things
        assert len(w) == 2
        assert issubclass(w[0].category, DeprecationWarning)
        assert "Use get_s3_filesystem instead" in str(w[0].message)
        assert issubclass(w[1].category, DeprecationWarning)
        assert "Use get_s3_filesystem instead" in str(w[1].message)
I ran into an error while running this test on my local machine which goes something like this FAILED tests/unit/test_deprecations.py::test_deprecation_warning - TypeError: Store.__init__() missing 1 required positional argument: 'auth'. I guess we need some access token here to be passed into the Store Class. What can be the next steps from here?
@Sherwin-14, nice job on coming up with the initial unit test for this. Will you be at today's hackday? If so, we can continue with this then.

Yes I would be there :)

CHANGELOG.md

chuckwondo · 2024-07-24T00:30:20Z

This is something I came up with for the unit test.

import warnings
import earthaccess 
from earthaccess import api 
from earthaccess.store import Store

def test_deprecation_warning():
    store = earthaccess.store.Store()
    with warnings.catch_warnings(record=True) as w:
        # Cause all warnings to always be triggered.
        warnings.simplefilter("always")
        # Trigger a warning.
        api.get_s3fs_session()
        store.get_s3fs_session()
        # Verify some things
        assert len(w) == 2
        assert issubclass(w[0].category, DeprecationWarning)
        assert "Use get_s3_filesystem instead" in str(w[0].message)
        assert issubclass(w[1].category, DeprecationWarning)
        assert "Use get_s3_filesystem instead" in str(w[1].message)

I ran into an error while running this test on my local machine which goes something like this FAILED tests/unit/test_deprecations.py::test_deprecation_warning - TypeError: Store.__init__() missing 1 required positional argument: 'auth'. I guess we need some access token here to be passed into the Store Class. What can be the next steps from here?

@Sherwin-14, thanks for joining today's hackday! Here's a summary of what we covered today:

You will attempt to resolve the rebasing issue you encountered. If this proves tricky, please reach out for help.
You will either parametrize the test function above, or simply split it into 2 test functions (since there are very few lines of code, if parametrization proves tricky, splitting into 2 test functions is perfectly acceptable)
You will resolve the TypeError shown above. After doing a bit more digging, I realized that we were looking at the wrong test_auth.py file. There are 2 of them: one in the unit tests and one in the integration tests. We were looking at the one in integration tests. To resolve the TypeError, do the following:

Within the same file where you put your new test code shown above, also add this:

@pytest.fixture(scope="session")
@responses.activate
@mock.patch("getpass.getpass")
@mock.patch("builtins.input")
def auth(user_input, user_password):
    user_input.return_value = "user"
    user_password.return_value = "password"
    json_response = [
        {"access_token": "EDL-token-1", "expiration_date": "12/15/2021"},
        {"access_token": "EDL-token-2", "expiration_date": "12/16/2021"},
    ]
    responses.add(
        responses.GET,
        "https://urs.earthdata.nasa.gov/api/users/tokens",
        json=json_response,
        status=200,
    )
    responses.add(
        responses.GET,
        "https://urs.earthdata.nasa.gov/profile",
        json={"email_address": "[email protected]"},
        status=200,
    )

    earthdata.login(strategy="interactive")

    return earthdata.__auth__

Then you should be able to modify your test like so:

def test_deprecation_warning(auth):
    store = earthaccess.store.Store(auth)
    ...

I think that should work, or at least get you much closer.

Co-authored-by: Chuck Daniels <[email protected]>

earthaccess/api.py

Co-authored-by: Chuck Daniels <[email protected]>

chuckwondo · 2024-07-24T13:11:10Z

@mfisher87, can you tell if the failing integration tests are related to the recent integration test troubles you've been working on recently?

Sherwin-14 · 2024-07-24T14:21:09Z

Hey just a small doubt. What is earthdata in the above code for the authentication. I am running into an attribute error and I am not sure what is earthdata in this case?

chuckwondo · 2024-07-24T14:27:17Z

Hey just a small doubt. What is earthdata in the above code for the authentication. I am running into an attribute error and I am not sure what is earthdata in this case?

It's a typo. Apologies. It should be earthaccess.

Sherwin-14 · 2024-07-24T14:29:47Z

Hey just a small doubt. What is earthdata in the above code for the authentication. I am running into an attribute error and I am not sure what is earthdata in this case?

It's a typo. Apologies. It should be earthaccess.

Thanks this helps! But then should'nt it be earthacess.api since we are using the login function from the api.py file?

chuckwondo · 2024-07-24T14:47:36Z

Hey just a small doubt. What is earthdata in the above code for the authentication. I am running into an attribute error and I am not sure what is earthdata in this case?

It's a typo. Apologies. It should be earthaccess.

Thanks this helps! But then should'nt it be earthacess.api since we are using the login function from the api.py file?

Not necessary, as the functions in api.py are exported at the top level.

Sherwin-14 · 2024-07-24T14:56:31Z

Hey just a small doubt. What is earthdata in the above code for the authentication. I am running into an attribute error and I am not sure what is earthdata in this case?

It's a typo. Apologies. It should be earthaccess.

Thanks this helps! But then should'nt it be earthacess.api since we are using the login function from the api.py file?

Not necessary, as the functions in api.py are exported at the top level.

I ran into this error while using earthacess instead of earthdata FAILED tests/unit/test_deprecations.py::test_deprecation_warning_for_store - ValueError: At least one of the concept_id, daac, provider or endpointparam...

This something different now where can we access daac and provider?

mfisher87 · 2024-07-24T15:24:00Z

@mfisher87, can you tell if the failing integration tests are related to the recent integration test troubles you've been working on recently?

Just closing the loop -- we think we need to set up credentials in the fork repo. More details in #773 #736

chuckwondo · 2024-07-24T15:29:35Z

Hey just a small doubt. What is earthdata in the above code for the authentication. I am running into an attribute error and I am not sure what is earthdata in this case?

It's a typo. Apologies. It should be earthaccess.

Thanks this helps! But then should'nt it be earthacess.api since we are using the login function from the api.py file?

Not necessary, as the functions in api.py are exported at the top level.

I ran into this error while using earthacess instead of earthdata FAILED tests/unit/test_deprecations.py::test_deprecation_warning_for_store - ValueError: At least one of the concept_id, daac, provider or endpointparam...

This something different now where can we access daac and provider?

Ah, okay. I think we can ignore the ValueError and still get the deprecation warning we expect. So instead of this line:

store.get_s3fs_session()

do this to suppress the ValueError:

with contextlib.suppress(ValueError):
    store.get_s3fs_session()

and at the top of the file, add this import:

import contextlib

chuckwondo · 2024-07-24T21:38:12Z

Just linking this PR to it's issue #766

Co-authored-by: Chuck Daniels <[email protected]>

Sherwin-14 · 2024-07-26T15:50:58Z

I have added unit test for deprecations

chuckwondo

@Sherwin-14, nicely done!

@mfisher87, regarding the failing integration tests, do you think that once this PR is merged that perhaps they will succeed because they should end up pulling the creds from the nsidc/earthaccess repo? Any harm in merging and trying?

mfisher87 · 2024-07-26T16:43:09Z

Awesome job @Sherwin-14 !

I do think they should pass on the main branch after a merge, because our repository credentials will be used. Would you be able to keep an eye on the tests after merge, and if they don't pass, open an issue?

Remember, the "10% rule" is still in place, so even if the integration tests say they passed, up to 10% may have failed. We really need to get on top of fixing that!

Sherwin-14 · 2024-07-26T16:47:35Z

Awesome job @Sherwin-14 !

I do think they should pass on the main branch after a merge, because our repository credentials will be used. Would you be able to keep an eye on the tests after merge, and if they don't pass, open an issue?

Remember, the "10% rule" is still in place, so even if the integration tests say they passed, up to 10% may have failed. We really need to get on top of fixing that!

So was my BASE repo theory right? I am kind of still confused this is quite complicated

mfisher87 · 2024-07-26T17:27:33Z

I agree it is quite complicated. I think at this point we really don't know the cause, but we expect things to work after merging to main because the complexity of the fork rules are out of the picture :)

chuckwondo · 2024-07-26T20:03:31Z

I agree it is quite complicated. I think at this point we really don't know the cause, but we expect things to work after merging to main because the complexity of the fork rules are out of the picture :)

@mfisher87, shall I merge? @Sherwin-14 are you okay if I do a squash merge?

mfisher87 · 2024-07-26T22:17:10Z

I'm good with a merge, but for whoever clicks merge: can you please keep an eye on the tests on the main branch to ensure they do pass? I am concerned that if they fail we may not notice right away.

Sherwin-14 · 2024-07-27T05:41:36Z

I agree it is quite complicated. I think at this point we really don't know the cause, but we expect things to work after merging to main because the complexity of the fork rules are out of the picture :)

@mfisher87, shall I merge? @Sherwin-14 are you okay if I do a squash merge?

Yeah I am okay with that

chuckwondo · 2024-07-27T18:05:01Z

Fixes #766

Sherwin-14 force-pushed the renaming branch from 3b1743d to 37ec438 Compare July 19, 2024 13:22

Sherwin-14 changed the title ~~Renamed get_s3fs_session with get_s3_filesystem"~~ Renamed get_s3fs_session with get_s3_filesystem Jul 19, 2024

Sherwin-14 force-pushed the renaming branch from 37ec438 to 1686e9c Compare July 19, 2024 13:25

chuckwondo requested changes Jul 19, 2024

View reviewed changes

Sherwin-14 force-pushed the renaming branch 2 times, most recently from ddcfb58 to 1a57e19 Compare July 20, 2024 05:41

Sherwin-14 changed the title ~~Renamed get_s3fs_session with get_s3_filesystem~~ Deprecated get_s3fs_session Jul 20, 2024

Sherwin-14 force-pushed the renaming branch from 1a57e19 to c323124 Compare July 20, 2024 05:52

Sherwin-14 requested a review from chuckwondo July 20, 2024 15:07

chuckwondo requested changes Jul 22, 2024

View reviewed changes

earthaccess/store.py Outdated Show resolved Hide resolved

earthaccess/store.py Show resolved Hide resolved

chuckwondo reviewed Jul 23, 2024

View reviewed changes

earthaccess/api.py Outdated Show resolved Hide resolved

earthaccess/store.py Outdated Show resolved Hide resolved

mfisher87 reviewed Jul 23, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Sherwin-14 force-pushed the renaming branch from cc54a65 to 6ce45e9 Compare July 23, 2024 16:02

Sherwin-14 added 2 commits July 24, 2024 15:55

Deprecated get_s3fs_session

9c01734

Made all the necessary changes in files

88f1f4a

Sherwin-14 force-pushed the renaming branch from 6ce45e9 to 88f1f4a Compare July 24, 2024 10:26

Apply suggestions from code review

2b270d5

Co-authored-by: Chuck Daniels <[email protected]>

chuckwondo reviewed Jul 24, 2024

View reviewed changes

earthaccess/api.py Outdated Show resolved Hide resolved

Update earthaccess/api.py as per suggestion

59255eb

Co-authored-by: Chuck Daniels <[email protected]>

chuckwondo mentioned this pull request Jul 24, 2024

Refactored the loop in the tests by parameterizing #736

Merged

chuckwondo mentioned this pull request Jul 24, 2024

Deprecate Store.get_s3fs_session, expose under new name: Store.get_s3_filesystem #766

Closed

Sherwin-14 force-pushed the renaming branch 2 times, most recently from 60fe156 to 40edbc4 Compare July 25, 2024 11:29

Update earthaccess/store.py as per suggestion

6733262

Co-authored-by: Chuck Daniels <[email protected]>

Sherwin-14 force-pushed the renaming branch from 40edbc4 to 6733262 Compare July 25, 2024 11:30

Sherwin-14 added 2 commits July 26, 2024 18:16

Unit test for deprecation

9394efd

Unit test for deprecation

bcdb0cc

Sherwin-14 requested review from chuckwondo and mfisher87 July 26, 2024 15:49

chuckwondo approved these changes Jul 26, 2024

View reviewed changes

chuckwondo merged commit 0945db5 into nsidc:main Jul 27, 2024
7 of 11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecated get_s3fs_session #768

Deprecated get_s3fs_session #768

Sherwin-14 commented Jul 19, 2024 •

edited

Loading

chuckwondo left a comment

Sherwin-14 commented Jul 19, 2024 •

edited

Loading

chuckwondo commented Jul 19, 2024 •

edited

Loading

mfisher87 commented Jul 19, 2024 •

edited

Loading

chuckwondo left a comment •

edited

Loading

Sherwin-14 commented Jul 22, 2024

chuckwondo commented Jul 22, 2024

chuckwondo commented Jul 22, 2024

Sherwin-14 commented Jul 23, 2024

Sherwin-14 commented Jul 23, 2024 •

edited

Loading

chuckwondo left a comment

chuckwondo commented Jul 23, 2024

chuckwondo commented Jul 23, 2024

Sherwin-14 commented Jul 23, 2024 •

edited

Loading

chuckwondo commented Jul 24, 2024

chuckwondo commented Jul 24, 2024

Sherwin-14 commented Jul 24, 2024

chuckwondo commented Jul 24, 2024

Sherwin-14 commented Jul 24, 2024

chuckwondo commented Jul 24, 2024

Sherwin-14 commented Jul 24, 2024 •

edited

Loading

mfisher87 commented Jul 24, 2024

chuckwondo commented Jul 24, 2024 •

edited

Loading

chuckwondo commented Jul 24, 2024

Sherwin-14 commented Jul 26, 2024

chuckwondo left a comment

mfisher87 commented Jul 26, 2024

Sherwin-14 commented Jul 26, 2024

mfisher87 commented Jul 26, 2024

chuckwondo commented Jul 26, 2024

mfisher87 commented Jul 26, 2024

Sherwin-14 commented Jul 27, 2024

chuckwondo commented Jul 27, 2024

Deprecated get_s3fs_session #768

Deprecated get_s3fs_session #768

Conversation

Sherwin-14 commented Jul 19, 2024 • edited Loading

chuckwondo left a comment

Choose a reason for hiding this comment

Sherwin-14 commented Jul 19, 2024 • edited Loading

chuckwondo commented Jul 19, 2024 • edited Loading

mfisher87 commented Jul 19, 2024 • edited Loading

chuckwondo left a comment • edited Loading

Choose a reason for hiding this comment

Sherwin-14 commented Jul 22, 2024

chuckwondo commented Jul 22, 2024

chuckwondo commented Jul 22, 2024

Sherwin-14 commented Jul 23, 2024

Sherwin-14 commented Jul 23, 2024 • edited Loading

chuckwondo left a comment

Choose a reason for hiding this comment

chuckwondo commented Jul 23, 2024

chuckwondo commented Jul 23, 2024

Sherwin-14 commented Jul 23, 2024 • edited Loading

chuckwondo commented Jul 24, 2024

chuckwondo commented Jul 24, 2024

Sherwin-14 commented Jul 24, 2024

chuckwondo commented Jul 24, 2024

Sherwin-14 commented Jul 24, 2024

chuckwondo commented Jul 24, 2024

Sherwin-14 commented Jul 24, 2024 • edited Loading

mfisher87 commented Jul 24, 2024

chuckwondo commented Jul 24, 2024 • edited Loading

chuckwondo commented Jul 24, 2024

Sherwin-14 commented Jul 26, 2024

chuckwondo left a comment

Choose a reason for hiding this comment

mfisher87 commented Jul 26, 2024

Sherwin-14 commented Jul 26, 2024

mfisher87 commented Jul 26, 2024

chuckwondo commented Jul 26, 2024

mfisher87 commented Jul 26, 2024

Sherwin-14 commented Jul 27, 2024

chuckwondo commented Jul 27, 2024

Sherwin-14 commented Jul 19, 2024 •

edited

Loading

Sherwin-14 commented Jul 19, 2024 •

edited

Loading

chuckwondo commented Jul 19, 2024 •

edited

Loading

mfisher87 commented Jul 19, 2024 •

edited

Loading

chuckwondo left a comment •

edited

Loading

Sherwin-14 commented Jul 23, 2024 •

edited

Loading

Sherwin-14 commented Jul 23, 2024 •

edited

Loading

Sherwin-14 commented Jul 24, 2024 •

edited

Loading

chuckwondo commented Jul 24, 2024 •

edited

Loading