Skip to content

CLDR-18518 abstracts: work around CORS, improve logging on ui #4615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

srl295
Copy link
Member

@srl295 srl295 commented Apr 16, 2025

CLDR-18518

  • This PR completes the ticket.

    • example, az-short https://st.unicode.org/cldr-apps/v#/ab/Languages_A_D/700f7be0e77ae39b had no abstract but az did
    • fix the error message on deferHelp (dbPedia extract)
    • add logError, logException, logInfo, logWarning to cldrNotify for low level datadog logging without showing a message popup (these will go to datadog)
    • cleanup some noise in initial gui load and ajax load (Browser debugger panel for network shows better information here anyway)
    • workaround CORS issue with dbpedia by adding the hostname of the request to the query, this way the cache is individual for, say, smoketest vs. production (vs local) - otherwise they aren't shareable.

ALLOW_MANY_COMMITS=true

srl295 added 2 commits April 16, 2025 12:42
- fix the error message on deferHelp (dbPedia extract)
- add logError, logException, logInfo, logWarning to cldrNotify for low level datadog logging without showing a message popup (these will go to datadog)
- cleanup some noise in initial gui load and ajax load (Browser debugger panel for network shows better information here anyway)
- workaround CORS issue with dbpedia by adding the hostname of the request to the query, this way the cache is individual for, say, smoketest vs. production (vs local) - otherwise they aren't shareable.
btangmu
btangmu previously approved these changes Apr 16, 2025
- add comment to clarify CORS issue
- restructure per code review
- use fetch() instead of jQuery
btangmu
btangmu previously approved these changes Apr 16, 2025
const url = new URL(endpoint);
url.searchParams.append("query", query);
url.searchParams.append("format", "JSON");
return fetch(url);
Copy link
Member

@btangmu btangmu Apr 16, 2025

Choose a reason for hiding this comment

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

I like to use cldrAjax.doFetch but I guess it's not appropriate for a non-unicode.org server. We might consider a new cldrAjax function for other servers. It's good to go thru a single module for http.

Copy link
Member Author

Choose a reason for hiding this comment

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

That would leak the session key! I would be fine with a generic fetch, it could even just be re-exported fetch() would that work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants