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

feat: new Zinnia API stationId #520

Merged
merged 3 commits into from
Apr 17, 2024

Conversation

PatrickNercessian
Copy link
Contributor

@PatrickNercessian PatrickNercessian commented Apr 12, 2024

DRAFT: DO NOT MERGE

Links:
space-meridian/roadmap#96

.gitignore Outdated
@@ -1 +1,2 @@
/target
*.DS_Store
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
*.DS_Store
*.DS_Store

@@ -178,6 +179,7 @@ mod tests {
cache_root: temp.join("cache").to_string_lossy().into(),
state_root: temp.join("state").to_string_lossy().into(),
wallet_address: "f1test".to_string(),
station_id: "zinnia-dev".to_string(),
Copy link
Member

Choose a reason for hiding this comment

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

I think that only passing through the environment variable "station_id" isn't ideal, as Zinnia ideally doesn't know anything about Station. Instead there should be a way to pass any key value pair into the module context. I believe that implementing this is out of scope for this PR, so just noting here that I would like to improve this in the future.

Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

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

Nice! The changes look great. 👏🏻

I'd like to change one minor detail; the rest LGTM. 👍🏻

Also let's wait for the CI results to ensure all is green.

daemon/args.rs Outdated Show resolved Hide resolved
@bajtos
Copy link
Member

bajtos commented Apr 15, 2024

@PatrickNercessian also please fix the build failures:

@PatrickNercessian PatrickNercessian changed the title Receive station_id from Core and push to module as an env var Feat: Receive station_id from Core and push to module as an env var Apr 15, 2024
@bajtos bajtos marked this pull request as ready for review April 17, 2024 07:27
@bajtos bajtos changed the title Feat: Receive station_id from Core and push to module as an env var feat: new Zinnia API stationId Apr 17, 2024
@bajtos
Copy link
Member

bajtos commented Apr 17, 2024

https://github.com/filecoin-station/zinnia/actions/runs/8692944647/job/23914200368?pr=520

Error: Unknown release type "Feat" found in pull request title "Feat: Receive station_id from Core and push to module as an env var". 

Available types:
 - feat
 - fix
 - chore
 - docs
 - deps
 - test
 - ci
 - refactor

To speed up the process, I changed the PR from "draft" to a regular one, updated the PR title and will proceed to land & publish this change.

@bajtos bajtos enabled auto-merge (squash) April 17, 2024 07:31
@bajtos bajtos merged commit f9b1eb3 into filecoin-station:main Apr 17, 2024
13 checks passed
@PatrickNercessian PatrickNercessian deleted the station_id branch April 30, 2024 15:14
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.

3 participants