fix: correct value for config variable UPLOAD_FOLDER
#25166
Conversation
| superset/static/assets | ||
| superset/static/assets/* | ||
| !superset/static/assets/.gitkeep | ||
| superset/static/uploads/* | ||
| !superset/static/uploads/.gitkeep |
There was a problem hiding this comment.
This checks both directories into git, thus being empty default directories. Any content will be ignored.
.gitignore
Outdated
| local_config.py | ||
| /superset_config.py | ||
| /superset_text.yml | ||
| superset.egg-info/ | ||
| superset/bin/supersetc | ||
| tmp | ||
| rat-results.txt | ||
| superset/app/ |
There was a problem hiding this comment.
This is the incorrect path that contained static/uploads/. superset/app/ shouldn't be a valid directory anyway. For developers, this change could potentially pop up affected files in their git repo as untracked files.
| UPLOAD_CHUNK_SIZE = 4096 | ||
|
|
||
| # The image upload folder, when using models with images | ||
| IMG_UPLOAD_FOLDER = BASE_DIR + "/app/static/uploads/" |
There was a problem hiding this comment.
I couldn't find any usage of these variables.
|
This PR is ready for review, but may require further discussion. I'm not sure, but that could quite possibly be a breaking change. Possibly one would have to move important files? Or a mention in the release notes is sufficient? |
sebastianliebscher
changed the title
UPLOAD_FOLDER
|
We're working toward 5.0, and I think that's the safe time to get this merged (finally). I've put it on the kanban board, just FYI :) Thanks for the contributions! |
michael-s-molina
added
the
risk:breaking-change
|
/korbit-review |
|
Closing and reopening to get CI to spin again. @sebastianliebscher can you please add an entry to UPDATING.md to help folks who are upgrading mitigate any risks here? |
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
I've completed my review and didn't find any issues.
Files scanned
| File Path | Reviewed |
|---|---|
| superset/config.py | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Need a new review? Comment
/korbit-reviewon this PR and I'll review your latest changes.Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-reviewcommand in a comment at the root of your PR.- You can ask Korbit to generate a new PR description using the
/korbit-generate-pr-descriptioncommand in any comment on your PR.- Too many Korbit comments? I can resolve all my comment threads if you use the
/korbit-resolvecommand in any comment on your PR.- Chat with Korbit on issues we post by tagging @korbit-ai in your reply.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Current Korbit Configuration
General Settings
Setting Value Review Schedule Automatic excluding drafts Max Issue Count 10 Automatic PR Descriptions ✅ Issue Categories
Category Enabled Naming ✅ Database Operations ✅ Documentation ✅ Logging ✅ Error Handling ✅ Systems and Environment ✅ Objects and Data Structures ✅ Readability and Maintainability ✅ Asynchronous Processing ✅ Design Patterns ✅ Third-Party Libraries ✅ Performance ✅ Security ✅ Functionality ✅ Feedback and Support
Note
Korbit Pro is free for open source projects 🎉
Looking to add Korbit to your team? Get started with a free 2 week trial here
SUMMARY
This PR fixes an incorrect default value for config variable
UPLOAD_FOLDER:superset/app/static/uploads/superset/static/uploads/Additionally:
static/uploads/is now checked into git (analog tosuperset/static/assets/)static/assets/was initially checked into git years before, but in another commit (also years before) then .gitignored without removal from git (I think unintentionally)The config variables
IMG_UPLOAD_FOLDERandIMG_UPLOAD_URLwere never used (found nothing using full text search) and are therefore removed.TESTING INSTRUCTIONS
docker build . -t testADDITIONAL INFORMATION