Skip to content
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

[maint] minor dev improvements #506

Merged
merged 3 commits into from
Apr 4, 2025
Merged

[maint] minor dev improvements #506

merged 3 commits into from
Apr 4, 2025

Conversation

PGijsbers
Copy link
Collaborator

@PGijsbers PGijsbers commented Mar 25, 2025

Change

Adds changes outline in #446:

  • Change default AIOD_LOGSTASH_PORT so it does not clash with MacOS defaults
  • Remove the use of sudo in the clean up script.
  • Add an extra reminder to the documentation that local files are not mounted unless USE_LOCAL_DEV is set.

How to Test

  • make sure you do not have AIOD_LOGSTASH_PORT set in your local override.
  • start containers
  • stop containers
  • invoke clean script

This should all work without issues and successfully empty the data directory similar to before the PR.

@Taniya-Das this is not high priority, but I'm asking you because it would be good to test this on a non-Mac machine.

Checklist

  • Tests have been added or updated to reflect the changes, or their absence is explicitly explained.
  • Documentation has been added or updated to reflect the changes, or their absence is explicitly explained.
  • A self-review has been conducted checking:
    • No unintended changes have been committed.
    • The changes in isolation seem reasonable.
    • Anything that may be odd or unintuitive is provided with a GitHub comment explaining it (but consider if this should not be a code comment or in the documentation instead).
  • All CI checks pass before pinging a reviewer, or provide an explanation if they do not.

Related Issues

closes #446

@PGijsbers PGijsbers requested a review from Taniya-Das March 25, 2025 12:27
@PGijsbers PGijsbers added enhancement New feature or request low priority Unlikely to get addressed anytime soon unless community shows significant interest. labels Mar 25, 2025
@PGijsbers PGijsbers moved this to In Progress in AIoD API Apr 1, 2025
@PGijsbers PGijsbers moved this from In Progress to Review in AIoD API Apr 1, 2025
@PGijsbers PGijsbers assigned Taniya-Das and PGijsbers and unassigned Taniya-Das Apr 1, 2025
Copy link
Collaborator

@Taniya-Das Taniya-Das left a comment

Choose a reason for hiding this comment

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

The test works well for me.

@PGijsbers PGijsbers merged commit 793a4b5 into develop Apr 4, 2025
1 check passed
@PGijsbers PGijsbers deleted the fix/446 branch April 4, 2025 09:08
@github-project-automation github-project-automation bot moved this from Review to Done in AIoD API Apr 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request low priority Unlikely to get addressed anytime soon unless community shows significant interest.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Minor dev improvements
2 participants