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

[HOTFIX] API Credential failed 문제로 다른 API로 변경 #104

Merged
merged 3 commits into from
Jan 31, 2024

Conversation

YoonyoungL
Copy link
Collaborator

@YoonyoungL YoonyoungL commented Jan 30, 2024

관련 이슈

🔒 Closes

#103

작업내용

추후 진행할 사항

  • Astoronomy API에서 해당 이슈를 해결한다면 원 API로 변경
  • 이슈 해결이 안된다면 Horizons API로 완전히 변경한 다음 폴더, 파일 정리

리뷰포인트

  • 임시로 변경한 Horizons API는 천체에 대한 정보를 한번에 받아오는 것이 안돼서 순서대로 요청 후 리스트에 저장해서 반환하는 형식으로 만들었습니다.
  • 임시 변경이여서 기존에 있던 코드에 추가해서 커밋합니다. 만약에 Credential 이슈가 지속된다면 정리해야할 것 같습니다.
  • 실제 천제 위치와 확인 부탁드립니다. (확인을 위해서 print문 아직 남겨놨습니다. 머지 전에 삭제 후 커밋 예정)

Reference

NASA Open APIs
Horizons API
Horizons System Manual

Checklist

  • 빌드를 위해 SceneDelegate 수정한 것 PR로 올리지 않았는지 확인
  • 필요없는 주석, 프린트문 제거했는지 확인
  • 컨벤션 지켰는지 확인

Copy link
Member

@lenamin lenamin left a comment

Choose a reason for hiding this comment

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

빠른 HOTFIX 감사합니다!

@@ -80,6 +77,7 @@ extension HorizonsAPIManager {
// Get time now and 1 minute later

let formatter = DateFormatter()
formatter.timeZone = TimeZone(identifier: "Europe/London")
Copy link
Member

Choose a reason for hiding this comment

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

타임존때문에 값이 낮인 것처럼 나왔던거군요!
받아오는 값이 정확히 일치합니다. 정말 고생하셨어요 제리!!!

@glitterer
Copy link
Collaborator

확인 마쳤습니다! 현재 미국 Providence, Rhode Island에서도 작동 잘하고 있습니다.
바쁘실텐데도 HOTFIX로 빠르게 고쳐주셔서 감사해요! 수고하셨습니다~~👍👍👍👍👍

Copy link
Collaborator

@glitterer glitterer left a comment

Choose a reason for hiding this comment

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

너무너무너무 고생많으셨습니다! 감사합니다👍👍👍
그리고 그냥 개인적으로 코드 보면서 궁금한 부분 살짝 코멘트 남겨서.. 혹~시 시간나시면 깔짝이라도 대답해주시면 감사하겠습니다 ㅎㅎ🙇‍♂️🙇‍♂️🙇‍♂️

@@ -41,3 +42,64 @@ extension AstronomyAPIManager {
return parameters
}
}

// MARK: - Horizons API
class HorizonsAPIManager: NetworkService {
Copy link
Collaborator

Choose a reason for hiding this comment

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

궁금해서 그런건데, 현재 이 horizons API에 대한 코드는 그럼 아예 새로 이 API에 대해서 작성을 새로 하신건가요..?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

기존에 있던 걸 참고해서 따로 만들었습니다!

Comment on lines +86 to +105
var command: String {
switch self {
case .sun:
return "10"
case .moon:
return "301"
case .mercury:
return "199"
case .venus:
return "299"
case .mars:
return "499"
case .jupiter:
return "599"
case .saturn:
return "699"
case .uranus:
return "799"
case .neptune:
return "899"
Copy link
Collaborator

Choose a reason for hiding this comment

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

바쁘실텐데 죄송합니다. 궁금해서 그런건데, 혹시 이 숫자들의 의미는 어디에서 오는건가요..? API에서 각 행성들에 대한 숫자인건가요? ㅎㅎ,,,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아 이건 설명을 못 썼네요.. 임시로 바꾼 API에서는 원하는 천체를 파라미터로 보내서 값을 요청하는데 그때 쓰는 숫자들입니다 API에서 각 행성들에 대한 숫자가 맞아요!

@YoonyoungL YoonyoungL merged commit dc15f7f into develop Jan 31, 2024
@YoonyoungL YoonyoungL deleted the 103-hotfix-change-api branch January 31, 2024 01:07
Copy link
Collaborator

@SohyeonKim-dev SohyeonKim-dev left a comment

Choose a reason for hiding this comment

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

제리 최고 , , 👩‍🚀🚀
API 관련 코드 작성하시느라 수고 많으셨어요..! 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔭 API API help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants