Skip to content

refactor: unify environment hashing on environment_content_hash#2190

Merged
kobe0938 merged 2 commits into
harbor-framework:mainfrom
kobe0938:unify-environment-content-hash
Jul 5, 2026
Merged

refactor: unify environment hashing on environment_content_hash#2190
kobe0938 merged 2 commits into
harbor-framework:mainfrom
kobe0938:unify-environment-content-hash

Conversation

@kobe0938

@kobe0938 kobe0938 commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

Replace environment_template_hash (dirhash-based) and environment_dir_hash/_truncated with environment_content_hash. Also remove the unused dockerfile_hash/dockerfile_hash_truncated pair as they are never used.

Hash suffixes are truncated to 12 characters (previously 8 for E2B, Novita, and Blaxel). 12 is much safer than 8; also make them consistent with Daytona and langsmith, which is 12. set a constant for it SNAPSHOT_HASH_LEN


Open in Devin Review

@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Enjoy a better diff viewing experience by clicking one of these URLs:

devin-ai-integration[bot]

This comment was marked as resolved.

@kobe0938 kobe0938 force-pushed the unify-environment-content-hash branch from 22867b5 to 8ea5501 Compare July 4, 2026 05:42

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

Open in Devin Review

Comment thread src/harbor/environments/definition.py
@kobe0938 kobe0938 force-pushed the unify-environment-content-hash branch from 8ea5501 to 5a1e859 Compare July 4, 2026 05:51
@xiaoxiangmoe

Copy link
Copy Markdown
Contributor

I think you can also let environment_dir_hash use docker_build_context_hash.

More details were considered in docker_build_context_hash, such as platform, build_args, stat.st_mode, etc.

@kobe0938

kobe0938 commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator Author

I think you can also let environment_dir_hash use docker_build_context_hash.

More details were considered in docker_build_context_hash, such as platform, build_args, stat.st_mode, etc.

Thanks for pointing out. Let's keep the docker_build_context_hash and environment_template_hash both at this point. This PR will focus on unifying/deprecating several functions.

Replace environment_template_hash (dirhash-based) and
environment_dir_hash/_truncated with environment_content_hash, and
remove the unused dockerfile_hash/dockerfile_hash_truncated pair.
Providers now derive artifact names from environment_id (a prefix of
the same digest), so environment_content_hash has a single caller:
BaseEnvironment.environment_id. DaytonaSnapshotService receives the
hash instead of computing it, and the unused _get_environment_hash /
_get_auto_snapshot_name helpers are removed.

Hash suffixes are standardized to SNAPSHOT_HASH_LEN = 12 characters
(previously 8 for E2B, Novita, and Blaxel). Derived artifact names
rotate once as a result: E2B/Novita template names, Blaxel image
names, Daytona auto_snapshot names, and LangSmith Dockerfile snapshot
names, so the first run after upgrading rebuilds each
template/snapshot. environment_id, local Docker image tags, and
user-provided snapshot/template names are unaffected. Transient files
(.DS_Store, .git, __pycache__) no longer perturb cache names.
@kobe0938 kobe0938 force-pushed the unify-environment-content-hash branch from 5a1e859 to d1e9aec Compare July 4, 2026 23:10
@kobe0938 kobe0938 merged commit 6fe8697 into harbor-framework:main Jul 5, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants