Skip to content

Qgov master 2.11.2 cicd uplift etag#218

Open
duttonw wants to merge 22 commits intoqgov-master-2.11.4from
qgov-master-2.11.2_cicd_uplift_etag
Open

Qgov master 2.11.2 cicd uplift etag#218
duttonw wants to merge 22 commits intoqgov-master-2.11.4from
qgov-master-2.11.2_cicd_uplift_etag

Conversation

@duttonw
Copy link
Member

@duttonw duttonw commented Mar 31, 2025

Fixes ckan#6955

Proposed fixes:

Introduce ETag response and ability to respond that data has not changed
Introduce ability to disable cache: private, and set no cache headers enforcement (in situations where Vary: Cookie fails)

Features:

  • includes tests covering changes
  • includes updated documentation
  • includes user-visible changes
  • includes API changes
  • includes bugfix for possible backport

Please [X] all the boxes above that apply

ThrawnCA and others added 13 commits February 6, 2025 12:34
…24372

- Show 'displayed name' for cosmetic name field
- Add more uploader functions to support plugins such as Archiver
- Add '/dataset' fallback to URL generation
- Convert dict values into strings before passing to Solr
- Avoid changing package modified timestamp when reordering resources
- Shorten the lock timeout on dropping datastore tables to avoid deadlocks
[QOLDEV-1087] update CKAN 2.11 fork base to 2.11.2 to patch CVE-2025-24372
…ail-addresses

[QOLSVC-9300] clarify that admins can see your email address
- Sysadmins can see emails on the edit form anyway, might as well make it easier
…ail-addresses

[QOLSVC-9300] allow sysadmins to view email addresses on profiles
* Merge all testing under one parent workflow
* Pytest now runs and is split into 12 to run in under 3 mins (vs 10min for only 4 workers)
* Allow dynamic splits to be passed in
* Retieve and store latest pytest time stats for even splits
* Allow individual workflows to still be callable (manually)
* Allow pytest to run in single instance mode to generate test duration time data for improved splits
* Uplift codeql-analysis workflow to v2
- Coveralls to Codecov
 * Coverage data submitted
 * Test units submited (pytest only at this time)
- Html Reports available to download
 * PyTest Test Results
 * Coverage Report
- Doc's generated via its own workflow (artifact has commit id in name)
@duttonw duttonw requested a review from ThrawnCA March 31, 2025 03:08
@duttonw duttonw self-assigned this Mar 31, 2025
@duttonw duttonw added the enhancement New feature or request label Mar 31, 2025
@codecov
Copy link

codecov bot commented Mar 31, 2025

❌ 3 Tests Failed:

Tests completed Failed Passed Skipped
3033 3 3030 3
View the top 3 failed test(s) by shortest run time
ckan/tests/views/test_cache.py::test_sets_private_cache_when_no_private_cache_env_present
Stack Traces | 0.931s run time
app = <ckan.tests.helpers.CKANTestApp object at 0x7fa28b1ee160>

    @pytest.mark.ckan_config("ckan.cache_expires", 7200)
    def test_sets_private_cache_when_no_private_cache_env_present(app: CKANTestApp):
        """Test that private cache is set when `__no_private_cache__` is not present but `__no_cache__` is present."""
        builder = EnvironBuilder(path='/', method='GET', headers={}, environ_overrides={
            "__no_cache__": True})
        env = builder.get_environ()
        Request(env)
        response = Response()  # dummy response
    
        with app.flask_app.request_context(env):  # only works if you have app.flask_app
            updated_response = views.set_cache_control_headers_for_response(response)
>       assert updated_response.cache_control.private is True
E       AssertionError: assert '*' is True
E        +  where '*' = <ResponseCacheControl private=None>.private
E        +    where <ResponseCacheControl private=None> = <Response 0 bytes [200 OK]>.cache_control

.../tests/views/test_cache.py:97: AssertionError
ckan/tests/views/test_cache.py::test_disables_cache_when_no_cache_env_present
Stack Traces | 0.936s run time
app = <ckan.tests.helpers.CKANTestApp object at 0x7f627ddc2ca0>

    @pytest.mark.ckan_config("ckan.cache_expires", 0)
    def test_disables_cache_when_no_cache_env_present(app: CKANTestApp):
        """Test that no-cache headers are set when `__no_cache__` is true and `__no_private_cache__` is true in the environment."""
        builder = EnvironBuilder(path='/', method='GET', headers={}, environ_overrides={
            "__no_cache__": True,
            '__no_private_cache__': True})
        env = builder.get_environ()
        Request(env)
        response = Response()  # dummy response
    
        with app.flask_app.request_context(env):  # only works if you have app.flask_app
            updated_response = views.set_cache_control_headers_for_response(response)
>       assert updated_response.cache_control.no_cache is True
E       AssertionError: assert '*' is True
E        +  where '*' = <ResponseCacheControl max-age='0' must-revalidate=None no-cache=None no-store=None>.no_cache
E        +    where <ResponseCacheControl max-age='0' must-revalidate=None no-cache=None no-store=None> = <Response 0 bytes [200 OK]>.cache_control

.../tests/views/test_cache.py:64: AssertionError
ckan/tests/views/test_cache.py::test_sets_private_cache_when___no_cache__is_set_and_no_private_cache_env_present
Stack Traces | 0.96s run time
app = <ckan.tests.helpers.CKANTestApp object at 0x7fdc71b57d00>

    @pytest.mark.ckan_config("ckan.cache_expires", 7200)
    def test_sets_private_cache_when___no_cache__is_set_and_no_private_cache_env_present(app: CKANTestApp):
        """Test that private cache is set when `__no_private_cache__` is absent but `__no_cache__` is present."""
        builder = EnvironBuilder(path='/', method='GET', headers={}, environ_overrides={
            "__no_cache__": True})
        env = builder.get_environ()
        Request(env)
        response = Response()  # dummy response
    
        with app.flask_app.request_context(env):  # only works if you have app.flask_app
            updated_response = views.set_cache_control_headers_for_response(response)
    
>       assert updated_response.cache_control.private is True
E       AssertionError: assert '*' is True
E        +  where '*' = <ResponseCacheControl private=None>.private
E        +    where <ResponseCacheControl private=None> = <Response 0 bytes [200 OK]>.cache_control

.../tests/views/test_cache.py:82: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

duttonw added 3 commits April 10, 2025 06:47
…e is never assigned in scope in helpers/jobs for now

./ckan/lib/helpers.py:2574:5: F824 `global _RESOURCE_FORMATS` is unused: name is never assigned in scope
    global _RESOURCE_FORMATS
    ^
./ckan/lib/helpers.py:2723:5: F824 `global helper_functions` is unused: name is never assigned in scope
    global helper_functions
    ^
./ckan/lib/jobs.py:105:5: F824 `global _queues` is unused: name is never assigned in scope
    global _queues
    ^
3     F824 `global _RESOURCE_FORMATS` is unused: name is never assigned in scope
@duttonw duttonw force-pushed the qgov-master-2.11.2_cicd_uplift_etag branch from 4639a66 to 0115317 Compare April 9, 2025 20:50
@duttonw
Copy link
Member Author

duttonw commented Apr 9, 2025

This is 2 commits on top of #217 which has work already included in upstream master

@duttonw duttonw marked this pull request as ready for review April 9, 2025 20:51
@duttonw duttonw requested review from ThrawnCA, ienxckl and zmacca April 9, 2025 20:51
@duttonw duttonw changed the base branch from qgov-master-2.11.2 to qgov-master-2.10.7 April 10, 2025 00:52
@duttonw duttonw changed the base branch from qgov-master-2.10.7 to qgov-master-2.11.2 April 10, 2025 00:52
@duttonw
Copy link
Member Author

duttonw commented Apr 10, 2025

backport from ckan upstream master: ckan#8781
need to fix a test that was highlighted that something was done incorrectly on enforcing attachments to only be downloaded except for pdf's which can be opened in the browser.

duttonw and others added 4 commits April 10, 2025 14:52
new config:
ckan.private_cache_expires: optional int
ckan.cache_private_enabled: default True
ckan.cache_etags: default True
ckan.cache_etags_notModified: default True

fix:
ckan.cache_enabled now treated as bool instead of just set
@duttonw duttonw force-pushed the qgov-master-2.11.2_cicd_uplift_etag branch from 0d0dc23 to 894cbf8 Compare April 10, 2025 04:54

@pytest.mark.ckan_config("ckan.cache_expires", 3600)
def test_sets_cache_control_headers_with__no_private_cache__set(app: CKANTestApp):
"""Test that cache control headers are set correctly when caching is allowed."""

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is exactly the same as the previous test. It should say something more specific to this test.

new config:
ckan.private_cache_expires: optional int
ckan.cache_private_enabled: default True
ckan.cache_etags: default True
ckan.cache_etags_notModified: default True

fix:
ckan.cache_enabled now treated as bool instead of just set
@ThrawnCA
Copy link

ThrawnCA commented Dec 1, 2025

@duttonw Can you uplift this to target 2.11.4?

@duttonw duttonw changed the base branch from qgov-master-2.11.2 to qgov-master-2.11.4 December 1, 2025 04:13
@duttonw
Copy link
Member Author

duttonw commented Dec 1, 2025

i'm going to need to redo this entire workload since upstream did not like the direction i took it :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Page visibility is cached beyond login/logout

2 participants