URL encode dataset names to support multibyte characters

[t0momi219]

Description

In projects containing models with names like the following, dataset creation fails, and an error occurs during execution.

└── dbt
    └── my_project
        └── models
            ├── 日本語名モデル.sql
            └── 日本語名モデル.yml

[2024-09-07T04:10:58.435+0000] {local.py:455} DEBUG - URIs to be converted to Dataset: ['snowflake://***.ap-northeast-1.aws/TEST_DB.TEST_SCHEMA.日本語名モデル']
[2024-09-07T04:10:58.437+0000] {taskinstance.py:3301} ERROR - Task failed with exception
Traceback (most recent call last):
  File "/home/airflow/.local/lib/python3.12/site-packages/cosmos/operators/local.py", line 459, in get_datasets
    datasets = [Dataset(uri) for uri in uris]
                ^^^^^^^^^^^^
  File "<attrs generated init airflow.datasets.Dataset>", line 3, in __init__
    _setattr('uri', __attr_converter_uri(uri))
                    ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/airflow/.local/lib/python3.12/site-packages/airflow/datasets/__init__.py", line 78, in _sanitize_uri
    raise ValueError("Dataset URI must only consist of ASCII characters")
ValueError: Dataset URI must only consist of ASCII characters

To support model names with multibyte characters, it might be good to URL encode the names.

Related Issue(s)

closes: #1197

Breaking Change?

Checklist

• I have made corresponding changes to the documentation (if required)
• I have added tests that prove my fix is effective or that my feature works

[netlify]

✅ Deploy Preview for sunny-pastelito-5ecb04 canceled.

Name	Link
🔨 Latest commit	10d1eae
🔍 Latest deploy log	https://app.netlify.com/sites/sunny-pastelito-5ecb04/deploys/66dc04010bb5aa0008a74e6c

[netlify]

✅ Deploy Preview for sunny-pastelito-5ecb04 ready!

Name	Link
🔨 Latest commit	3b6ac62
🔍 Latest deploy log	https://app.netlify.com/sites/sunny-pastelito-5ecb04/deploys/66f56d5aaf48540008404054
😎 Deploy Preview	https://deploy-preview-1198--sunny-pastelito-5ecb04.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.95%. Comparing base (11de5ba) to head (3b6ac62).
Report is 5 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1198   +/-   ##
=======================================
  Coverage   95.95%   95.95%           
=======================================
  Files          64       64           
  Lines        3656     3657    +1     
=======================================
+ Hits         3508     3509    +1     
  Misses        148      148           

Flag	Coverage Δ
	95.95% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tatiana]

Hi @t0momi219 this contribution is very valuable, thank you for working on this!

Please, as suggested by @pankajkoti , could you add a test case?
It would be great if we could have a test that didn't pass this change, when calling get_datasets, and that it passes after this change. This would help make sure future changes in Cosmos won't undo this fix.
The right place is probably: https://github.com/astronomer/astronomer-cosmos/blob/main/tests/operators/test_local.py to confirm

Also, in the PR description, you refer the original issue #1197. Could you add before it one of the following keywords, so Github will automatically close the issue once the PR is merged?

close
closes
closed
fix
fixes
fixed
resolve
resolves
resolved

Once we have the test, we can merge this change. If we can do this during this week, we can release it as part of Cosmos 1.7.

[t0momi219]

@pankajkoti @tatiana
Thank you for your review. I plan to implement a test by placing a model with a multibyte character name in the dev/dags/dbt/simple/ project. If there is a more appropriate directory where this model should be placed, please let me know.
I will write the test by tomorrow at the latest.
Thanks!

[tatiana]

Hi @t0momi219 , thank you very much! Sounds like a good plan!

[tatiana]

Thank you very much, @t0momi219 , for identifying, reporting and fixing this issue. I'm sure many members of the community will be able to benefit from this.

Thanks also for adding the missing test.

I made one last comment inline, if you could address it before we merge this PR, it would be awesome! Thank you very much.

[pankajkoti]

LGTM, thanks @t0momi219 . Will keep an eye on the CI run & once it succeeds, I will go ahead to merge it